[Gluster-devel] [PATCH v9] vfs_glusterfs: Samba VFS module for glusterfs

[Anand Avati]

On 5/29/13 1:19 PM, Jeremy Allison wrote:
> On Wed, May 29, 2013 at 12:23:04PM -0700, Anand Avati wrote:
>>
>> We just uncovered this issue in our QE testing -
>>
>> On Wed, May 29, 2013 at 4:21 AM, Anand Avati <avati at redhat.com> wrote:
>> +static DIR *vfs_gluster_fdopendir(struct vfs_handle_struct *handle,
>> +                                 files_struct *fsp, const char *mask,
>> +                                 uint32 attributes)
>> +{
>> +       return (DIR *) glfd_fd_get(fsp->fh->fd);
>> +}
>>
>> When code takes this vfs_fdopendir() path (happened when testing
>> fsstress, most of the times vfs_opendir() is called - not sure why),
>> we are just passing a pointer of glfs_fd structure ...
>
> The vfs_fdopendir() will get called when SMB2 is being
> used and a directory listing is being done on an open
> SMB2 directory handle.
>
>> +static int vfs_gluster_closedir(struct vfs_handle_struct *handle,
>> DIR *dirp)
>> +{
>> +       return glfs_closedir((void *)dirp);
>> +}
>>
>> ... and Samba does a vfs_closedir() _and_ vfs_close(), essentially
>> doing a double free on the glfs_fd structure.
>
> Let me look into the mainline Samba code here. As I recall
> it should set the fsp->fh->fd to -1 after doing the vfs_closedir()
> when the file handle on the directory is closed. Thus the
> vfs_close should see the fsp->fh->fd as already zero and
> just ignore it.
>


Our testing was on a backport of this patch in v3-6-stable, which has 
this interesting piece of code:

void dptr_CloseDir(files_struct *fsp)
{
         if (fsp->dptr) {
/* 
 

  * Ugly hack. We have defined fdopendir to return ENOSYS if dirfd also 
isn't 

  * present. I hate Solaris. JRA. 
 

  */
#ifdef HAVE_DIRFD
                 if (fsp->fh->fd != -1 &&
                                 fsp->dptr->dir_hnd &&
                                 dirfd(fsp->dptr->dir_hnd->dir)) {
                         /* The call below closes the underlying fd. */
                         fsp->fh->fd = -1;
                 }
#endif
                 dptr_close_internal(fsp->dptr);
                 fsp->dptr = NULL;
         }
}


1. The code is calling dirfd() on the value returned by vfs_fdopendir() 
and vfs_opendir(). We return 'struct glfs_fd' typecasted to DIR * 
(technique I copied from vfs_ceph). So dirfd() is getting called against 
'struct glfs_fd *' in this case.

2. The logic around dirfd() is checking for > 0 as the condition of a 
valid fd. Since dirfd is now working on a differnet object, it probably 
picked an arbitrary location in glfs_fd and found the value to be 0, and 
this lead to fsp->fh->fd to _not_ be 0.

3. In our test case the fd number returned by vfs_gluster for the 
initial open file _was_ 0. That dirfd() condition needs a fix, I think?

I don't know if the master branch has equivalent code elsewhere, 
couldn't find it easily..

Avati