From e764e00446a653de68e7fa0d85bf7861657f66a9 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 8 Apr 2013 15:11:28 -0700 Subject: [PATCH 01/10] Change source3/modules/vfs_dirsort.c from MALLOC -> TALLOC. Signed-off-by: Jeremy Allison Reviewed-by: Andreas Schneider --- source3/modules/vfs_dirsort.c | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/source3/modules/vfs_dirsort.c b/source3/modules/vfs_dirsort.c index adeab04..88191b0 100644 --- a/source3/modules/vfs_dirsort.c +++ b/source3/modules/vfs_dirsort.c @@ -37,12 +37,7 @@ struct dirsort_privates { }; static void free_dirsort_privates(void **datap) { - struct dirsort_privates *data = (struct dirsort_privates *) *datap; - SAFE_FREE(data->directory_list); - SAFE_FREE(data); - *datap = NULL; - - return; + TALLOC_FREE(*datap); } static bool open_and_sort_dir (vfs_handle_struct *handle) @@ -71,9 +66,10 @@ static bool open_and_sort_dir (vfs_handle_struct *handle) SMB_VFS_NEXT_REWINDDIR(handle, data->source_directory); /* Set up an array and read the directory entries into it */ - SAFE_FREE(data->directory_list); /* destroy previous cache if needed */ - data->directory_list = (SMB_STRUCT_DIRENT *)SMB_MALLOC( - data->number_of_entries * sizeof(SMB_STRUCT_DIRENT)); + TALLOC_FREE(data->directory_list); /* destroy previous cache if needed */ + data->directory_list = talloc_zero_array(data, + SMB_STRUCT_DIRENT, + data->number_of_entries); if (!data->directory_list) { return false; } @@ -97,9 +93,7 @@ static SMB_STRUCT_DIR *dirsort_opendir(vfs_handle_struct *handle, struct dirsort_privates *data = NULL; /* set up our private data about this directory */ - data = (struct dirsort_privates *)SMB_MALLOC( - sizeof(struct dirsort_privates)); - + data = talloc_zero(handle->conn, struct dirsort_privates); if (!data) { return NULL; } @@ -132,9 +126,7 @@ static SMB_STRUCT_DIR *dirsort_fdopendir(vfs_handle_struct *handle, struct dirsort_privates *data = NULL; /* set up our private data about this directory */ - data = (struct dirsort_privates *)SMB_MALLOC( - sizeof(struct dirsort_privates)); - + data = talloc_zero(handle->conn, struct dirsort_privates); if (!data) { return NULL; } @@ -147,7 +139,7 @@ static SMB_STRUCT_DIR *dirsort_fdopendir(vfs_handle_struct *handle, attr); if (data->source_directory == NULL) { - SAFE_FREE(data); + TALLOC_FREE(data); return NULL; } -- 1.8.1.4 From f4c4b4589055d741e707ca48bb52ef82064c9cec Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 8 Apr 2013 16:31:53 -0700 Subject: [PATCH 02/10] Protect against early error in SMB_VFS_NEXT_READDIR. Signed-off-by: Jeremy Allison Reviewed-by: Andreas Schneider --- source3/modules/vfs_dirsort.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/source3/modules/vfs_dirsort.c b/source3/modules/vfs_dirsort.c index 88191b0..7185e5b 100644 --- a/source3/modules/vfs_dirsort.c +++ b/source3/modules/vfs_dirsort.c @@ -61,6 +61,10 @@ static bool open_and_sort_dir (vfs_handle_struct *handle) data->number_of_entries++; } + if (data->number_of_entries == 0) { + return false; + } + /* Open the underlying directory and count the number of entries Skip back to the beginning as we'll read it again */ SMB_VFS_NEXT_REWINDDIR(handle, data->source_directory); -- 1.8.1.4 From 7e96c44bd8f2ee2ca6ea85da4f39f8b09586babd Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 8 Apr 2013 16:38:03 -0700 Subject: [PATCH 03/10] Use an index i rather than re-using a state variable. Signed-off-by: Jeremy Allison Reviewed-by: Andreas Schneider --- source3/modules/vfs_dirsort.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/source3/modules/vfs_dirsort.c b/source3/modules/vfs_dirsort.c index 7185e5b..a3d106d 100644 --- a/source3/modules/vfs_dirsort.c +++ b/source3/modules/vfs_dirsort.c @@ -44,7 +44,7 @@ static bool open_and_sort_dir (vfs_handle_struct *handle) { SMB_STRUCT_DIRENT *dp; struct stat dir_stat; - long current_pos; + unsigned int i; struct dirsort_privates *data = NULL; SMB_VFS_HANDLE_GET_DATA(handle, data, struct dirsort_privates, @@ -77,15 +77,13 @@ static bool open_and_sort_dir (vfs_handle_struct *handle) if (!data->directory_list) { return false; } - current_pos = data->pos; - data->pos = 0; + i = 0; while ((dp = SMB_VFS_NEXT_READDIR(handle, data->source_directory, NULL)) != NULL) { - data->directory_list[data->pos++] = *dp; + data->directory_list[i++] = *dp; } /* Sort the directory entries by name */ - data->pos = current_pos; TYPESAFE_QSORT(data->directory_list, data->number_of_entries, compare_dirent); return true; } -- 1.8.1.4 From f121190f13f070ac49160db14d6a20247002b29c Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 8 Apr 2013 16:40:35 -0700 Subject: [PATCH 04/10] Protect open_and_sort_dir() from the directory changing size. Otherwise there could be an error between initial count, allocation and re-read. Signed-off-by: Jeremy Allison Reviewed-by: Andreas Schneider --- source3/modules/vfs_dirsort.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/source3/modules/vfs_dirsort.c b/source3/modules/vfs_dirsort.c index a3d106d..3b7f57a 100644 --- a/source3/modules/vfs_dirsort.c +++ b/source3/modules/vfs_dirsort.c @@ -30,7 +30,7 @@ static int compare_dirent (const SMB_STRUCT_DIRENT *da, const SMB_STRUCT_DIRENT struct dirsort_privates { long pos; SMB_STRUCT_DIRENT *directory_list; - long number_of_entries; + unsigned int number_of_entries; time_t mtime; SMB_STRUCT_DIR *source_directory; int fd; @@ -42,9 +42,9 @@ static void free_dirsort_privates(void **datap) { static bool open_and_sort_dir (vfs_handle_struct *handle) { - SMB_STRUCT_DIRENT *dp; struct stat dir_stat; unsigned int i; + unsigned int total_count = 0; struct dirsort_privates *data = NULL; SMB_VFS_HANDLE_GET_DATA(handle, data, struct dirsort_privates, @@ -58,10 +58,10 @@ static bool open_and_sort_dir (vfs_handle_struct *handle) while (SMB_VFS_NEXT_READDIR(handle, data->source_directory, NULL) != NULL) { - data->number_of_entries++; + total_count++; } - if (data->number_of_entries == 0) { + if (total_count == 0) { return false; } @@ -73,16 +73,22 @@ static bool open_and_sort_dir (vfs_handle_struct *handle) TALLOC_FREE(data->directory_list); /* destroy previous cache if needed */ data->directory_list = talloc_zero_array(data, SMB_STRUCT_DIRENT, - data->number_of_entries); + total_count); if (!data->directory_list) { return false; } - i = 0; - while ((dp = SMB_VFS_NEXT_READDIR(handle, data->source_directory, - NULL)) != NULL) { - data->directory_list[i++] = *dp; + for (i = 0; i < total_count; i++) { + SMB_STRUCT_DIRENT *dp = SMB_VFS_NEXT_READDIR(handle, + data->source_directory, + NULL); + if (dp == NULL) { + break; + } + data->directory_list[i] = *dp; } + data->number_of_entries = i; + /* Sort the directory entries by name */ TYPESAFE_QSORT(data->directory_list, data->number_of_entries, compare_dirent); return true; -- 1.8.1.4 From 22b222594be73e5a1bcd2a4efaf2f03568a25251 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 9 Apr 2013 10:29:47 -0700 Subject: [PATCH 05/10] Clean error paths in opendir and fd_opendir by only setting handle data on success. Pass extra struct dirsort_privates * to open_and_sort_dir() function to avoid it having to re-read the handle data. Signed-off-by: Jeremy Allison Reviewed-by: Andreas Schneider --- source3/modules/vfs_dirsort.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/source3/modules/vfs_dirsort.c b/source3/modules/vfs_dirsort.c index 3b7f57a..82b96eb 100644 --- a/source3/modules/vfs_dirsort.c +++ b/source3/modules/vfs_dirsort.c @@ -40,7 +40,8 @@ static void free_dirsort_privates(void **datap) { TALLOC_FREE(*datap); } -static bool open_and_sort_dir (vfs_handle_struct *handle) +static bool open_and_sort_dir(vfs_handle_struct *handle, + struct dirsort_privates *data) { struct stat dir_stat; unsigned int i; @@ -115,14 +116,15 @@ static SMB_STRUCT_DIR *dirsort_opendir(vfs_handle_struct *handle, data->fd = dirfd(data->source_directory); - SMB_VFS_HANDLE_SET_DATA(handle, data, free_dirsort_privates, - struct dirsort_privates, return NULL); - - if (!open_and_sort_dir(handle)) { + if (!open_and_sort_dir(handle, data)) { SMB_VFS_NEXT_CLOSEDIR(handle,data->source_directory); + TALLOC_FREE(data); return NULL; } + SMB_VFS_HANDLE_SET_DATA(handle, data, free_dirsort_privates, + struct dirsort_privates, return NULL); + return data->source_directory; } @@ -153,16 +155,17 @@ static SMB_STRUCT_DIR *dirsort_fdopendir(vfs_handle_struct *handle, data->fd = dirfd(data->source_directory); - SMB_VFS_HANDLE_SET_DATA(handle, data, free_dirsort_privates, - struct dirsort_privates, return NULL); - - if (!open_and_sort_dir(handle)) { + if (!open_and_sort_dir(handle, data)) { SMB_VFS_NEXT_CLOSEDIR(handle,data->source_directory); + TALLOC_FREE(data); /* fd is now closed. */ fsp->fh->fd = -1; return NULL; } + SMB_VFS_HANDLE_SET_DATA(handle, data, free_dirsort_privates, + struct dirsort_privates, return NULL); + return data->source_directory; } @@ -185,7 +188,7 @@ static SMB_STRUCT_DIRENT *dirsort_readdir(vfs_handle_struct *handle, /* throw away cache and re-read the directory if we've changed */ if (current_mtime > data->mtime) { - open_and_sort_dir(handle); + open_and_sort_dir(handle, data); } if (data->pos >= data->number_of_entries) { -- 1.8.1.4 From da15b9fea0fbbe8db9f08f98a069a6008391e9dd Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 9 Apr 2013 10:38:24 -0700 Subject: [PATCH 06/10] Check SMB_VFS_NEXT_OPENDIR return in dirsort_opendir(). Signed-off-by: Jeremy Allison Reviewed-by: Andreas Schneider --- source3/modules/vfs_dirsort.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/source3/modules/vfs_dirsort.c b/source3/modules/vfs_dirsort.c index 82b96eb..25a8cac 100644 --- a/source3/modules/vfs_dirsort.c +++ b/source3/modules/vfs_dirsort.c @@ -114,6 +114,11 @@ static SMB_STRUCT_DIR *dirsort_opendir(vfs_handle_struct *handle, data->source_directory = SMB_VFS_NEXT_OPENDIR(handle, fname, mask, attr); + if (data->source_directory == NULL) { + TALLOC_FREE(data); + return NULL; + } + data->fd = dirfd(data->source_directory); if (!open_and_sort_dir(handle, data)) { -- 1.8.1.4 From 10e9b37c417e9a5e50e065319988c8844d412b2d Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 9 Apr 2013 10:43:53 -0700 Subject: [PATCH 07/10] Convert mtime from a time_t to a struct timespec. In preparation for removing the dirfd and using fsp_stat() and VFS_STAT functions. Signed-off-by: Jeremy Allison Reviewed-by: Andreas Schneider --- source3/modules/vfs_dirsort.c | 41 ++++++++++++++++++++++++++--------------- 1 file changed, 26 insertions(+), 15 deletions(-) diff --git a/source3/modules/vfs_dirsort.c b/source3/modules/vfs_dirsort.c index 25a8cac..ea65e63 100644 --- a/source3/modules/vfs_dirsort.c +++ b/source3/modules/vfs_dirsort.c @@ -31,7 +31,7 @@ struct dirsort_privates { long pos; SMB_STRUCT_DIRENT *directory_list; unsigned int number_of_entries; - time_t mtime; + struct timespec mtime; SMB_STRUCT_DIR *source_directory; int fd; }; @@ -40,21 +40,35 @@ static void free_dirsort_privates(void **datap) { TALLOC_FREE(*datap); } +static bool get_sorted_dir_mtime(vfs_handle_struct *handle, + struct dirsort_privates *data, + struct timespec *ret_mtime) +{ + int ret; + struct stat dir_stat; + + ret = fstat(data->fd, &dir_stat); + + if (ret == -1) { + return false; + } + + ret_mtime->tv_sec = dir_stat.st_mtime; + ret_mtime->tv_nsec = 0; + + return true; +} + static bool open_and_sort_dir(vfs_handle_struct *handle, struct dirsort_privates *data) { - struct stat dir_stat; - unsigned int i; + unsigned int i = 0; unsigned int total_count = 0; - struct dirsort_privates *data = NULL; - - SMB_VFS_HANDLE_GET_DATA(handle, data, struct dirsort_privates, - return false); data->number_of_entries = 0; - if (fstat(data->fd, &dir_stat) == 0) { - data->mtime = dir_stat.st_mtime; + if (get_sorted_dir_mtime(handle, data, &data->mtime) == false) { + return false; } while (SMB_VFS_NEXT_READDIR(handle, data->source_directory, NULL) @@ -179,20 +193,17 @@ static SMB_STRUCT_DIRENT *dirsort_readdir(vfs_handle_struct *handle, SMB_STRUCT_STAT *sbuf) { struct dirsort_privates *data = NULL; - time_t current_mtime; - struct stat dir_stat; + struct timespec current_mtime; SMB_VFS_HANDLE_GET_DATA(handle, data, struct dirsort_privates, return NULL); - if (fstat(data->fd, &dir_stat) == -1) { + if (get_sorted_dir_mtime(handle, data, ¤t_mtime) == false) { return NULL; } - current_mtime = dir_stat.st_mtime; - /* throw away cache and re-read the directory if we've changed */ - if (current_mtime > data->mtime) { + if (timespec_compare(¤t_mtime, &data->mtime) > 1) { open_and_sort_dir(handle, data); } -- 1.8.1.4 From 2fdacb8cea673e8cb132e5c316e323a1d18a87f8 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 9 Apr 2013 10:50:55 -0700 Subject: [PATCH 08/10] Remove the use of dirfd inside the vfs_dirsort.c. Signed-off-by: Jeremy Allison Reviewed-by: Andreas Schneider --- source3/modules/vfs_dirsort.c | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/source3/modules/vfs_dirsort.c b/source3/modules/vfs_dirsort.c index ea65e63..9b772cc 100644 --- a/source3/modules/vfs_dirsort.c +++ b/source3/modules/vfs_dirsort.c @@ -33,7 +33,8 @@ struct dirsort_privates { unsigned int number_of_entries; struct timespec mtime; SMB_STRUCT_DIR *source_directory; - int fd; + files_struct *fsp; /* If open via FDOPENDIR. */ + struct smb_filename *smb_fname; /* If open via OPENDIR */ }; static void free_dirsort_privates(void **datap) { @@ -45,16 +46,21 @@ static bool get_sorted_dir_mtime(vfs_handle_struct *handle, struct timespec *ret_mtime) { int ret; - struct stat dir_stat; + struct timespec mtime; - ret = fstat(data->fd, &dir_stat); + if (data->fsp) { + ret = fsp_stat(data->fsp); + mtime = data->fsp->fsp_name->st.st_ex_mtime; + } else { + ret = SMB_VFS_STAT(handle->conn, data->smb_fname); + mtime = data->smb_fname->st.st_ex_mtime; + } if (ret == -1) { return false; } - ret_mtime->tv_sec = dir_stat.st_mtime; - ret_mtime->tv_nsec = 0; + *ret_mtime = mtime; return true; } @@ -113,6 +119,7 @@ static SMB_STRUCT_DIR *dirsort_opendir(vfs_handle_struct *handle, const char *fname, const char *mask, uint32 attr) { + NTSTATUS status; struct dirsort_privates *data = NULL; /* set up our private data about this directory */ @@ -124,6 +131,16 @@ static SMB_STRUCT_DIR *dirsort_opendir(vfs_handle_struct *handle, data->directory_list = NULL; data->pos = 0; + status = create_synthetic_smb_fname(data, + fname, + NULL, + NULL, + &data->smb_fname); + if (!NT_STATUS_IS_OK(status)) { + TALLOC_FREE(data); + return NULL; + } + /* Open the underlying directory and count the number of entries */ data->source_directory = SMB_VFS_NEXT_OPENDIR(handle, fname, mask, attr); @@ -133,8 +150,6 @@ static SMB_STRUCT_DIR *dirsort_opendir(vfs_handle_struct *handle, return NULL; } - data->fd = dirfd(data->source_directory); - if (!open_and_sort_dir(handle, data)) { SMB_VFS_NEXT_CLOSEDIR(handle,data->source_directory); TALLOC_FREE(data); @@ -162,6 +177,7 @@ static SMB_STRUCT_DIR *dirsort_fdopendir(vfs_handle_struct *handle, data->directory_list = NULL; data->pos = 0; + data->fsp = fsp; /* Open the underlying directory and count the number of entries */ data->source_directory = SMB_VFS_NEXT_FDOPENDIR(handle, fsp, mask, @@ -172,8 +188,6 @@ static SMB_STRUCT_DIR *dirsort_fdopendir(vfs_handle_struct *handle, return NULL; } - data->fd = dirfd(data->source_directory); - if (!open_and_sort_dir(handle, data)) { SMB_VFS_NEXT_CLOSEDIR(handle,data->source_directory); TALLOC_FREE(data); -- 1.8.1.4 From 8a612bc3714b3eb4c3da32bff47f46805fee52c6 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 9 Apr 2013 11:02:58 -0700 Subject: [PATCH 09/10] Remove unneeded initializations (we already talloc_zero). Signed-off-by: Jeremy Allison Reviewed-by: Andreas Schneider --- source3/modules/vfs_dirsort.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/source3/modules/vfs_dirsort.c b/source3/modules/vfs_dirsort.c index 9b772cc..698e96b 100644 --- a/source3/modules/vfs_dirsort.c +++ b/source3/modules/vfs_dirsort.c @@ -128,9 +128,6 @@ static SMB_STRUCT_DIR *dirsort_opendir(vfs_handle_struct *handle, return NULL; } - data->directory_list = NULL; - data->pos = 0; - status = create_synthetic_smb_fname(data, fname, NULL, @@ -175,8 +172,6 @@ static SMB_STRUCT_DIR *dirsort_fdopendir(vfs_handle_struct *handle, return NULL; } - data->directory_list = NULL; - data->pos = 0; data->fsp = fsp; /* Open the underlying directory and count the number of entries */ -- 1.8.1.4 From b19ec632aee12ceb9bcc77b9f64aaa4058caec0f Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 9 Apr 2013 16:56:24 -0700 Subject: [PATCH 10/10] Ensure we test the dirsort module in make test. Signed-off-by: Jeremy Allison Reviewed-by: Andreas Schneider Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Thu Apr 11 21:17:21 CEST 2013 on sn-devel-104 --- selftest/target/Samba3.pm | 2 ++ 1 file changed, 2 insertions(+) diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index 864f3dc..01a1c47 100644 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -650,6 +650,8 @@ sub provision($$$$$$) print CONF " [tmp] path = $shrdir + comment = smb username is [%U] + vfs objects = $vfs_modulesdir_abs/dirsort.so [tmpguest] path = $shrdir guest ok = yes -- 1.8.1.4