From 36158965c7f0c7fcbf1f3aaf264a553d7f1fa650 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 --- source3/modules/vfs_dirsort.c | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/source3/modules/vfs_dirsort.c b/source3/modules/vfs_dirsort.c index 98472f8..8139556 100644 --- a/source3/modules/vfs_dirsort.c +++ b/source3/modules/vfs_dirsort.c @@ -37,10 +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; + TALLOC_FREE(*datap); } static bool open_and_sort_dir (vfs_handle_struct *handle) @@ -69,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 = (struct dirent *)SMB_MALLOC( - data->number_of_entries * sizeof(struct dirent)); + TALLOC_FREE(data->directory_list); /* destroy previous cache if needed */ + data->directory_list = talloc_zero_array(data, + struct dirent, + data->number_of_entries); if (!data->directory_list) { return false; } @@ -95,9 +93,7 @@ static 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; } @@ -130,9 +126,7 @@ static 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; } @@ -145,7 +139,7 @@ static DIR *dirsort_fdopendir(vfs_handle_struct *handle, attr); if (data->source_directory == NULL) { - SAFE_FREE(data); + TALLOC_FREE(data); return NULL; } -- 1.8.1.3 From 0217d5c69a2aeae302dc1aef84c7dda160c989fd 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 --- 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 8139556..6fe7c18 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.3 From bac1a12bcf71557f9f48b647677be2d19f5991c5 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 --- 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 6fe7c18..1c24bc9 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) { 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.3 From 1560479e7622268ef54c4521d8c74899f1a81663 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 --- 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 1c24bc9..3ea720f 100644 --- a/source3/modules/vfs_dirsort.c +++ b/source3/modules/vfs_dirsort.c @@ -30,7 +30,7 @@ static int compare_dirent (const struct dirent *da, const struct dirent *db) struct dirsort_privates { long pos; struct dirent *directory_list; - long number_of_entries; + unsigned int number_of_entries; time_t mtime; 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) { - 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, 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++) { + 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.3 From 1c98a9c2e523144f9e7c403ab4077dbda6e1a2ff 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 --- 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 3ea720f..d6f19b5 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 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 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 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.3 From fbe27563a56b5ecae294e8bd0c667d0f1243284c 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 --- 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 d6f19b5..e2c61da 100644 --- a/source3/modules/vfs_dirsort.c +++ b/source3/modules/vfs_dirsort.c @@ -114,6 +114,11 @@ static 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.3 From d294f86347039d7a4eab57f0bd5e5125dd9d1aa5 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 --- 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 e2c61da..3388d53 100644 --- a/source3/modules/vfs_dirsort.c +++ b/source3/modules/vfs_dirsort.c @@ -31,7 +31,7 @@ struct dirsort_privates { long pos; struct dirent *directory_list; unsigned int number_of_entries; - time_t mtime; + struct timespec mtime; 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 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.3 From e6eb2b445d01550290f388c7eb2e0ebfcb8f362e 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 --- source3/modules/vfs_dirsort.c | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/source3/modules/vfs_dirsort.c b/source3/modules/vfs_dirsort.c index 3388d53..4741239 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; 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; - - ret = fstat(data->fd, &dir_stat); + struct timespec *pmtime; + + if (data->fsp) { + ret = fsp_stat(data->fsp); + pmtime = &data->fsp->fsp_name->st.st_ex_mtime; + } else { + ret = SMB_VFS_STAT(handle->conn, data->smb_fname); + pmtime = &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 = *pmtime; return true; } @@ -113,6 +119,7 @@ static 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 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 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 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 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.3 From 24304180816a8f1bc80759ad27fe6d88ca4db6bf 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 --- 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 4741239..adb2614 100644 --- a/source3/modules/vfs_dirsort.c +++ b/source3/modules/vfs_dirsort.c @@ -128,9 +128,6 @@ static 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 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.3 From 25ee52dac928fbeb7f2151b60d01dfcdcf76af7a 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 --- selftest/target/Samba3.pm | 1 + 1 file changed, 1 insertion(+) diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index 72c1116..1b14f1c 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -1026,6 +1026,7 @@ sub provision($$$$$$) path = $shrdir comment = encrypt smb username is [%U] smb encrypt = required + vfs objects = $vfs_modulesdir_abs/dirsort.so [tmpguest] path = $shrdir guest ok = yes -- 1.8.1.3