[PATCH] Fix bug #9778 - Samba directory code uses dirfd() without vectoring through a VFS call.
Jeremy Allison
jra at samba.org
Thu Apr 11 13:59:07 MDT 2013
Here is a patchset that removes the remaining uses
of dirfd() from smbd (in conjunction with the patchset
that went into master for bug #9777 this removes
all dirfd() calls).
This will fix Samba VFS modules like the vfs_scannedonly.c
code that expected to be able to return an opaque pointer
cast to a DIR *, and also the (not yet merged) CEPH
filesystem VFS module.
Once this goes in (and into 4.0.x) it will fix the
VFS ABI breakage we had in 4.0.x by using a non-VFS
vectored dirfd() call. We can then have the discussion
for 4.1.0 if we want to either add a VFS_DIRFD() call
for 4.1, or disallow it completely by changing the
return from VFS_OPENDIR/VFS_FDOPENDIR from DIR *
to a typedef'ed void * instead.
Please review and push if you're happy.
Cheers,
Jeremy.
-------------- next part --------------
From 15c25476d94db1feda9774aeb17375eff65a6151 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 10 Apr 2013 16:21:39 -0700
Subject: [PATCH 1/4] Maintain a back-pointer to the fsp in struct smb_Dir when
opening with FDOPENDIR.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/dir.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c
index f1c177f..e623316 100644
--- a/source3/smbd/dir.c
+++ b/source3/smbd/dir.c
@@ -50,6 +50,8 @@ struct smb_Dir {
struct name_cache_entry *name_cache;
unsigned int name_cache_index;
unsigned int file_number;
+ files_struct *fsp; /* Back pointer to containing fsp, only
+ set from OpenDir_fsp(). */
};
struct dptr_struct {
@@ -1540,7 +1542,9 @@ static struct smb_Dir *OpenDir_fsp(TALLOC_CTX *mem_ctx, connection_struct *conn,
if (fsp->is_directory && fsp->fh->fd != -1) {
dirp->dir = SMB_VFS_FDOPENDIR(fsp, mask, attr);
- if (dirp->dir == NULL) {
+ if (dirp->dir != NULL) {
+ dirp->fsp = fsp;
+ } else {
DEBUG(10,("OpenDir_fsp: SMB_VFS_FDOPENDIR on %s returned "
"NULL (%s)\n",
dirp->dir_path,
--
1.8.1.3
From 929a21434ba181e268d7abf2ee6163990f0443f8 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 10 Apr 2013 16:24:15 -0700
Subject: [PATCH 2/4] In the struct smb_Dir destructor, use the fsp back
pointer to release resources.
Removes one use of dirfd().
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/dir.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c
index e623316..a38f16f 100644
--- a/source3/smbd/dir.c
+++ b/source3/smbd/dir.c
@@ -1448,17 +1448,20 @@ bool is_visible_file(connection_struct *conn, const char *dir_path,
static int smb_Dir_destructor(struct smb_Dir *dirp)
{
if (dirp->dir) {
-#ifdef HAVE_DIRFD
- if (dirp->conn->sconn) {
- files_struct *fsp = file_find_fd(dirp->conn->sconn,
- dirfd(dirp->dir));
- if (fsp) {
- /* The call below closes the underlying fd. */
- fsp->fh->fd = -1;
+ SMB_VFS_CLOSEDIR(dirp->conn,dirp->dir);
+ if (dirp->fsp) {
+ /*
+ * The SMB_VFS_CLOSEDIR above
+ * closes the underlying fd inside
+ * dirp->fsp.
+ */
+ dirp->fsp->fh->fd = -1;
+ if (dirp->fsp->dptr) {
+ SMB_ASSERT(dirp->fsp->dptr->dir_hnd == dirp);
+ dirp->fsp->dptr->dir_hnd = NULL;
}
+ dirp->fsp = NULL;
}
-#endif
- SMB_VFS_CLOSEDIR(dirp->conn,dirp->dir);
}
if (dirp->conn->sconn && !dirp->conn->sconn->using_smb2) {
dirp->conn->sconn->searches.dirhandles_open--;
--
1.8.1.3
From 7ea68d22577c5d2ce19310a5400d71412341c5aa Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 10 Apr 2013 16:29:03 -0700
Subject: [PATCH 3/4] Remove the "Ugly hack" that was the second use of
dirfd().
The destructor does all the resource deallocation needed.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/dir.c | 18 +++++-------------
1 file changed, 5 insertions(+), 13 deletions(-)
diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c
index a38f16f..3587abe 100644
--- a/source3/smbd/dir.c
+++ b/source3/smbd/dir.c
@@ -679,20 +679,12 @@ done:
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
+ /*
+ * The destructor for the struct smb_Dir
+ * (fsp->dptr->dir_hnd) now handles
+ * all resource deallocation.
+ */
dptr_close_internal(fsp->dptr);
- fsp->dptr = NULL;
}
}
--
1.8.1.3
From 886830a40474069d8c2cfb73dba2194a56afc6ca Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 10 Apr 2013 16:30:10 -0700
Subject: [PATCH 4/4] Remove dependency on detection of HAVE_DIRFD for use of
fdopendir().
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/lib/system.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/source3/lib/system.c b/source3/lib/system.c
index d69f1c6..8dbf7dc 100644
--- a/source3/lib/system.c
+++ b/source3/lib/system.c
@@ -634,13 +634,11 @@ void kernel_flock(int fd, uint32 share_mode, uint32 access_mask)
/*******************************************************************
An fdopendir wrapper.
- Ugly hack - we need dirfd for this to work correctly in the
- calling code.. JRA.
********************************************************************/
DIR *sys_fdopendir(int fd)
{
-#if defined(HAVE_FDOPENDIR) && defined(HAVE_DIRFD)
+#if defined(HAVE_FDOPENDIR)
return fdopendir(fd);
#else
errno = ENOSYS;
--
1.8.1.3
More information about the samba-technical
mailing list