|
From: | Anand Avati |
Subject: | Re: [Gluster-devel] [PATCH v9] vfs_glusterfs: Samba VFS module for glusterfs |
Date: | Wed, 29 May 2013 13:54:20 -0700 |
User-agent: | Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:17.0) Gecko/20130509 Thunderbird/17.0.6 |
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 <address@hidden> 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
[Prev in Thread] | Current Thread | [Next in Thread] |