The branch, master has been updated via dc3ace6 Fix the waf build with the new "cleans up stale processes" test. via 182face s3: Check for serverid_exists in close_directory via 2db3ecb s3: Check for serverid_exists in close_remove_share_mode via 4329609 s3: Be less picky on stale share mode entries via 49237b0 s3: Check for serverid_exists in find_oplock_types Signed-off-by: Jeremy Allison <j...@samba.org> via e34b730 s3: Test whether get_share_mode_lock cleans up stale processes via 4e1656a s3: Check for serverid_exists in rename_share_filename via 6379709 s3: Do not check the PIDs is parse_share_modes via 5017bbe s3: Check for serverid_exists in smb_posix_unlink via 4962ab2 s3: Check for serverid_exists in open_mode_check via 689a04b s3: Check for serverid_exists in notify_deferred_opens via f45966d s3: Add "share_mode_stale_server" from 918eb3e s4:torture: add smb2.session.expire1
http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master - Log ----------------------------------------------------------------- commit dc3ace63d4ebeed82b8ca9ae8e44fece6a887710 Author: Jeremy Allison <j...@samba.org> Date: Wed May 16 17:16:17 2012 -0700 Fix the waf build with the new "cleans up stale processes" test. Autobuild-User: Jeremy Allison <j...@samba.org> Autobuild-Date: Fri May 18 00:20:36 CEST 2012 on sn-devel-104 commit 182faceab21000f3103f030b31cab5baba6deb6c Author: Volker Lendecke <v...@samba.org> Date: Mon May 7 15:23:29 2012 +0200 s3: Check for serverid_exists in close_directory Signed-off-by: Jeremy Allison <j...@samba.org> commit 2db3ecbc954a8e56d9c83aacd5967817cbcb2394 Author: Volker Lendecke <v...@samba.org> Date: Mon May 7 15:23:29 2012 +0200 s3: Check for serverid_exists in close_remove_share_mode Signed-off-by: Jeremy Allison <j...@samba.org> commit 43296090f93c48aab061c0fd4ed295a0eb64756b Author: Volker Lendecke <v...@samba.org> Date: Mon May 14 14:57:34 2012 +0200 s3: Be less picky on stale share mode entries If a process died, the share mode entry might be bogus. Ignore those entries. Signed-off-by: Jeremy Allison <j...@samba.org> commit 49237b0cef181f51dc584c8f2b0a718c5a6a97e5 Author: Volker Lendecke <v...@samba.org> Date: Wed May 16 16:51:26 2012 -0700 s3: Check for serverid_exists in find_oplock_types Signed-off-by: Jeremy Allison <j...@samba.org> commit e34b7306fc421741727f20275b9dd4878906ba05 Author: Volker Lendecke <v...@samba.org> Date: Fri May 11 14:39:42 2012 +0200 s3: Test whether get_share_mode_lock cleans up stale processes Signed-off-by: Jeremy Allison <j...@samba.org> commit 4e1656a78265efec3b56c3bf410d7d257b409821 Author: Volker Lendecke <v...@samba.org> Date: Mon May 7 15:23:10 2012 +0200 s3: Check for serverid_exists in rename_share_filename Signed-off-by: Jeremy Allison <j...@samba.org> commit 6379709b88501ea09dff52236d87fab97accbab6 Author: Volker Lendecke <v...@samba.org> Date: Mon May 7 16:34:11 2012 +0200 s3: Do not check the PIDs is parse_share_modes We do that when conflicts arise Signed-off-by: Jeremy Allison <j...@samba.org> commit 5017bbe70d2e40e1942224291789793330a31605 Author: Volker Lendecke <v...@samba.org> Date: Mon May 7 15:23:29 2012 +0200 s3: Check for serverid_exists in smb_posix_unlink Signed-off-by: Jeremy Allison <j...@samba.org> commit 4962ab2aa870d0de2f3488b4bd3cf14d7c987c9a Author: Volker Lendecke <v...@samba.org> Date: Mon May 7 15:23:10 2012 +0200 s3: Check for serverid_exists in open_mode_check Signed-off-by: Jeremy Allison <j...@samba.org> commit 689a04bc6c21d35cf2b3e025cb9ff3aab3fcfc23 Author: Volker Lendecke <v...@samba.org> Date: Mon May 7 12:22:50 2012 +0200 s3: Check for serverid_exists in notify_deferred_opens We will remove the check in parse_share_modes soon Signed-off-by: Jeremy Allison <j...@samba.org> commit f45966d16919481e3caa1f723c244269da21a28f Author: Volker Lendecke <v...@samba.org> Date: Mon May 7 12:57:07 2012 +0200 s3: Add "share_mode_stale_server" This is a helper routine that prunes a dead share mode entry on demand. This prepares for removing the serverids_exist call in parse_share_modes. Signed-off-by: Jeremy Allison <j...@samba.org> ----------------------------------------------------------------------- Summary of changes: source3/Makefile.in | 3 +- source3/locking/locking.c | 40 ++++++++- source3/locking/proto.h | 1 + source3/locking/share_mode_lock.c | 42 --------- source3/smbd/close.c | 16 +++- source3/smbd/open.c | 25 +++++ source3/smbd/trans2.c | 3 + source3/torture/proto.h | 1 + source3/torture/test_cleanup.c | 175 +++++++++++++++++++++++++++++++++++++ source3/torture/torture.c | 1 + source3/wscript_build | 1 + 11 files changed, 262 insertions(+), 46 deletions(-) Changeset truncated at 500 lines: diff --git a/source3/Makefile.in b/source3/Makefile.in index 486ec72..a401543 100644 --- a/source3/Makefile.in +++ b/source3/Makefile.in @@ -1280,11 +1280,12 @@ SMBTORTURE_OBJ1 = torture/torture.o torture/nbio.o torture/scanner.o torture/uta torture/t_strappend.o SMBTORTURE_OBJ = $(SMBTORTURE_OBJ1) $(PARAM_OBJ) $(TLDAP_OBJ) \ - $(LIBSMB_OBJ) $(KRBCLIENT_OBJ) $(LIB_NONSMBD_OBJ) \ + $(LIBSMB_OBJ) $(KRBCLIENT_OBJ) $(LIB_NONSMBD_OBJ) $(LOCKING_OBJ) \ @LIBWBCLIENT_STATIC@ \ torture/wbc_async.o \ ../nsswitch/wb_reqtrans.o \ ../libcli/lsarpc/util_lsarpc.o \ + lib/filename_util.o \ $(LIBMSRPC_OBJ) $(LIBMSRPC_GEN_OBJ) $(LIBCLI_ECHO_OBJ) MASKTEST_OBJ = torture/masktest.o $(PARAM_OBJ) $(LIBSMB_OBJ) $(KRBCLIENT_OBJ) \ diff --git a/source3/locking/locking.c b/source3/locking/locking.c index b9afd23..b9fba17 100644 --- a/source3/locking/locking.c +++ b/source3/locking/locking.c @@ -556,6 +556,10 @@ bool rename_share_filename(struct messaging_context *msg_ctx, continue; } + if (share_mode_stale_pid(d, i)) { + continue; + } + DEBUG(10,("rename_share_filename: sending rename message to " "pid %s file_id %s sharepath %s base_name %s " "stream_name %s\n", @@ -616,7 +620,9 @@ bool is_valid_share_mode_entry(const struct share_mode_entry *e) num_props += (EXCLUSIVE_OPLOCK_TYPE(e->op_type) ? 1 : 0); num_props += (LEVEL_II_OPLOCK_TYPE(e->op_type) ? 1 : 0); - SMB_ASSERT(num_props <= 1); + if (serverid_exists(&e->pid) && (num_props > 1)) { + smb_panic("Invalid share mode entry"); + } return (num_props != 0); } @@ -625,6 +631,38 @@ bool is_deferred_open_entry(const struct share_mode_entry *e) return (e->op_type == DEFERRED_OPEN_ENTRY); } +/* + * In case d->share_modes[i] conflicts with something or otherwise is + * being used, we need to make sure the corresponding process still + * exists. This routine checks it and potentially removes the entry + * from d->share_modes. Modifies d->num_share_modes, watch out in + * routines iterating over that array. + */ +bool share_mode_stale_pid(struct share_mode_data *d, unsigned i) +{ + struct share_mode_entry *e; + + if (i > d->num_share_modes) { + DEBUG(1, ("Asking for index %u, only %u around\n", + i, (unsigned)d->num_share_modes)); + return false; + } + e = &d->share_modes[i]; + if (serverid_exists(&e->pid)) { + DEBUG(10, ("PID %s (index %u out of %u) still exists\n", + procid_str_static(&e->pid), i, + (unsigned)d->num_share_modes)); + return false; + } + DEBUG(10, ("PID %s (index %u out of %u) does not exist anymore\n", + procid_str_static(&e->pid), i, + (unsigned)d->num_share_modes)); + *e = d->share_modes[d->num_share_modes-1]; + d->num_share_modes -= 1; + d->modified = true; + return true; +} + /******************************************************************* Fill a share mode entry. ********************************************************************/ diff --git a/source3/locking/proto.h b/source3/locking/proto.h index 54badd9..f6a6f2e 100644 --- a/source3/locking/proto.h +++ b/source3/locking/proto.h @@ -168,6 +168,7 @@ void get_file_infos(struct file_id id, struct timespec *write_time); bool is_valid_share_mode_entry(const struct share_mode_entry *e); bool is_deferred_open_entry(const struct share_mode_entry *e); +bool share_mode_stale_pid(struct share_mode_data *d, unsigned i); void set_share_mode(struct share_mode_lock *lck, files_struct *fsp, uid_t uid, uint64_t mid, uint16 op_type); void add_deferred_open(struct share_mode_lock *lck, uint64_t mid, diff --git a/source3/locking/share_mode_lock.c b/source3/locking/share_mode_lock.c index f28332c..1fea748 100644 --- a/source3/locking/share_mode_lock.c +++ b/source3/locking/share_mode_lock.c @@ -118,9 +118,6 @@ static struct share_mode_data *parse_share_modes(TALLOC_CTX *mem_ctx, const TDB_DATA dbuf) { struct share_mode_data *d; - int i; - struct server_id *pids; - bool *pid_exists; enum ndr_err_code ndr_err; DATA_BLOB blob; @@ -148,45 +145,6 @@ static struct share_mode_data *parse_share_modes(TALLOC_CTX *mem_ctx, NDR_PRINT_DEBUG(share_mode_data, d); } - /* - * Ensure that each entry has a real process attached. - */ - - pids = talloc_array(talloc_tos(), struct server_id, - d->num_share_modes); - if (pids == NULL) { - DEBUG(0, ("talloc failed\n")); - goto fail; - } - pid_exists = talloc_array(talloc_tos(), bool, d->num_share_modes); - if (pid_exists == NULL) { - DEBUG(0, ("talloc failed\n")); - goto fail; - } - - for (i=0; i<d->num_share_modes; i++) { - pids[i] = d->share_modes[i].pid; - } - if (!serverids_exist(pids, d->num_share_modes, pid_exists)) { - DEBUG(0, ("serverid_exists failed\n")); - goto fail; - } - - i = 0; - while (i < d->num_share_modes) { - struct share_mode_entry *e = &d->share_modes[i]; - if (!pid_exists[i]) { - DEBUG(10, ("wipe non-existent pid %s\n", - procid_str_static(&e->pid))); - *e = d->share_modes[d->num_share_modes-1]; - d->num_share_modes -= 1; - d->modified = True; - continue; - } - i += 1; - } - TALLOC_FREE(pid_exists); - TALLOC_FREE(pids); return d; fail: TALLOC_FREE(d); diff --git a/source3/smbd/close.c b/source3/smbd/close.c index c87b1a0..e46e1ae 100644 --- a/source3/smbd/close.c +++ b/source3/smbd/close.c @@ -179,9 +179,15 @@ static void notify_deferred_opens(struct smbd_server_connection *sconn, num_deferred = 0; for (i=0; i<lck->data->num_share_modes; i++) { - if (is_deferred_open_entry(&lck->data->share_modes[i])) { - num_deferred += 1; + struct share_mode_entry *e = &lck->data->share_modes[i]; + + if (!is_deferred_open_entry(e)) { + continue; + } + if (share_mode_stale_pid(lck->data, i)) { + continue; } + num_deferred += 1; } if (num_deferred == 0) { return; @@ -419,6 +425,9 @@ static NTSTATUS close_remove_share_mode(files_struct *fsp, if (fsp->posix_open && (e->flags & SHARE_MODE_FLAG_POSIX_OPEN)) { continue; } + if (share_mode_stale_pid(lck->data, i)) { + continue; + } delete_file = False; break; } @@ -1069,6 +1078,9 @@ static NTSTATUS close_directory(struct smb_request *req, files_struct *fsp, if (fsp->posix_open && (e->flags & SHARE_MODE_FLAG_POSIX_OPEN)) { continue; } + if (share_mode_stale_pid(lck->data, i)) { + continue; + } delete_dir = False; break; } diff --git a/source3/smbd/open.c b/source3/smbd/open.c index 543a661..f0e523d 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -1024,6 +1024,11 @@ static NTSTATUS open_mode_check(connection_struct *conn, * too */ if (share_conflict(&lck->data->share_modes[i], access_mask, share_access)) { + + if (share_mode_stale_pid(lck->data, i)) { + continue; + } + return NT_STATUS_SHARING_VIOLATION; } } @@ -1123,6 +1128,11 @@ static void find_oplock_types(files_struct *fsp, if (BATCH_OPLOCK_TYPE(lck->data->share_modes[i].op_type)) { /* batch - can only be one. */ + if (share_mode_stale_pid(lck->data, i)) { + DEBUG(10, ("find_oplock_types: Found stale " + "batch oplock\n")); + continue; + } if (*pp_ex_or_batch || *pp_batch || *got_level2 || *got_no_oplock) { smb_panic("Bad batch oplock entry."); } @@ -1130,6 +1140,11 @@ static void find_oplock_types(files_struct *fsp, } if (EXCLUSIVE_OPLOCK_TYPE(lck->data->share_modes[i].op_type)) { + if (share_mode_stale_pid(lck->data, i)) { + DEBUG(10, ("find_oplock_types: Found stale " + "duplicate oplock\n")); + continue; + } /* Exclusive or batch - can only be one. */ if (*pp_ex_or_batch || *got_level2 || *got_no_oplock) { smb_panic("Bad exclusive or batch oplock entry."); @@ -1139,6 +1154,11 @@ static void find_oplock_types(files_struct *fsp, if (LEVEL_II_OPLOCK_TYPE(lck->data->share_modes[i].op_type)) { if (*pp_batch || *pp_ex_or_batch) { + if (share_mode_stale_pid(lck->data, i)) { + DEBUG(10, ("find_oplock_types: Found " + "stale LevelII oplock\n")); + continue; + } smb_panic("Bad levelII oplock entry."); } *got_level2 = true; @@ -1146,6 +1166,11 @@ static void find_oplock_types(files_struct *fsp, if (lck->data->share_modes[i].op_type == NO_OPLOCK) { if (*pp_batch || *pp_ex_or_batch) { + if (share_mode_stale_pid(lck->data, i)) { + DEBUG(10, ("find_oplock_types: Found " + "stale NO_OPLOCK entry\n")); + continue; + } smb_panic("Bad no oplock entry."); } *got_no_oplock = true; diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index 590ee5b..936b39a 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -7595,6 +7595,9 @@ static NTSTATUS smb_posix_unlink(connection_struct *conn, if (e->flags & SHARE_MODE_FLAG_POSIX_OPEN) { continue; } + if (share_mode_stale_pid(lck->data, i)) { + continue; + } /* Fail with sharing violation. */ close_file(req, fsp, NORMAL_CLOSE); TALLOC_FREE(lck); diff --git a/source3/torture/proto.h b/source3/torture/proto.h index 80618ce..4104fbc 100644 --- a/source3/torture/proto.h +++ b/source3/torture/proto.h @@ -104,6 +104,7 @@ bool run_local_conv_auth_info(int dummy); bool run_local_sprintf_append(int dummy); bool run_cleanup1(int dummy); bool run_cleanup2(int dummy); +bool run_cleanup3(int dummy); bool run_ctdb_conn(int dummy); bool run_msg_test(int dummy); bool run_notify_bench2(int dummy); diff --git a/source3/torture/test_cleanup.c b/source3/torture/test_cleanup.c index 39f579a..d9dce40 100644 --- a/source3/torture/test_cleanup.c +++ b/source3/torture/test_cleanup.c @@ -18,11 +18,14 @@ */ #include "includes.h" +#include "locking/proto.h" #include "torture/proto.h" #include "system/filesys.h" +#include "system/select.h" #include "libsmb/libsmb.h" #include "libcli/smb/smbXcli_base.h" #include "libcli/security/security.h" +#include "librpc/gen_ndr/open_files.h" bool run_cleanup1(int dummy) { @@ -154,3 +157,175 @@ bool run_cleanup2(int dummy) } return true; } + +static bool create_stale_share_mode_entry(const char *fname, + struct file_id *p_id) +{ + struct cli_state *cli; + uint16_t fnum; + NTSTATUS status; + SMB_STRUCT_STAT sbuf; + struct file_id id; + + if (!torture_open_connection(&cli, 0)) { + return false; + } + + status = torture_setup_unix_extensions(cli); + if (!NT_STATUS_IS_OK(status)) { + printf("torture_setup_unix_extensions failed: %s\n", + nt_errstr(status)); + return false; + } + status = cli_openx(cli, fname, O_RDWR|O_CREAT, DENY_ALL, &fnum); + if (!NT_STATUS_IS_OK(status)) { + printf("open of %s failed (%s)\n", fname, nt_errstr(status)); + return false; + } + status = cli_posix_stat(cli, fname, &sbuf); + if (!NT_STATUS_IS_OK(status)) { + printf("cli_posix_stat failed: %s\n", nt_errstr(status)); + return false; + } + status = smbXcli_conn_samba_suicide(cli->conn, 1); + if (!NT_STATUS_IS_OK(status)) { + printf("smbXcli_conn_samba_suicide failed: %s\n", + nt_errstr(status)); + return false; + } + + id.devid = sbuf.st_ex_rdev; + id.inode = sbuf.st_ex_ino; + id.extid = 0; + + poll(NULL, 0, 1000); + + *p_id = id; + return true; +} + +static bool corrupt_dummy(struct share_mode_data *d) +{ + return true; +} + +static bool invalidate_sharemode(struct share_mode_data *d) +{ + d->share_modes[0].op_type = + OPLOCK_EXCLUSIVE|OPLOCK_BATCH|OPLOCK_LEVEL_II; + d->modified = true; + return true; +} + +static bool duplicate_entry(struct share_mode_data *d, int i) +{ + struct share_mode_entry *tmp; + + if (i >= d->num_share_modes) { + return false; + } + + tmp = talloc_realloc(d, d->share_modes, struct share_mode_entry, + d->num_share_modes + 1); + if (tmp == NULL) { + return false; + } + d->share_modes = tmp; + d->num_share_modes += 1; + d->share_modes[d->num_share_modes-1] = d->share_modes[i]; + d->modified = true; + return true; +} + +static bool create_duplicate_batch(struct share_mode_data *d) +{ + if (d->num_share_modes != 1) { + return false; + } + d->share_modes[0].op_type = OPLOCK_BATCH; + if (!duplicate_entry(d, 0)) { + return false; + } + return true; +} + +struct corruption_fns { + bool (*fn)(struct share_mode_data *d); + const char *descr; +}; + +bool run_cleanup3(int dummy) +{ + struct cli_state *cli; + const char *fname = "cleanup3"; + uint16_t fnum; + NTSTATUS status; + struct share_mode_lock *lck; + struct file_id id; + size_t i; + + struct corruption_fns fns[] = { + { corrupt_dummy, "no corruption" }, + { invalidate_sharemode, "invalidate_sharemode" }, + { create_duplicate_batch, "create_duplicate_batch" }, + }; + + printf("CLEANUP3: Checking that a share mode is cleaned up on " + "conflict\n"); + + for (i=0; i<ARRAY_SIZE(fns); i++) { + + printf("testing %s\n", fns[i].descr); + + if (!create_stale_share_mode_entry(fname, &id)) { + printf("create_stale_entry failed\n"); + return false; + } + + printf("%d %d %d\n", (int)id.devid, (int)id.inode, + (int)id.extid); + + if (!locking_init()) { + printf("locking_init failed\n"); + return false; + } + lck = get_existing_share_mode_lock(talloc_tos(), id); + if (lck == NULL) { + printf("get_existing_share_mode_lock failed\n"); + return false; + } + if (lck->data->num_share_modes != 1) { + printf("get_existing_share_mode_lock did clean up\n"); + return false; + } + + fns[i].fn(lck->data); + + TALLOC_FREE(lck); + + if (!torture_open_connection(&cli, 0)) { + return false; + } + status = cli_openx(cli, fname, O_RDWR|O_CREAT, DENY_ALL, + &fnum); + if (!NT_STATUS_IS_OK(status)) { + printf("open of %s failed (%s)\n", fname, + nt_errstr(status)); + return false; + } + lck = get_existing_share_mode_lock(talloc_tos(), id); + if (lck == NULL) { + printf("get_existing_share_mode_lock failed\n"); + return false; + } + if (lck->data->num_share_modes != 1) { + printf("conflicting open did not clean up\n"); + return false; + } + TALLOC_FREE(lck); + + torture_close_connection(cli); + } + + return true; +} diff --git a/source3/torture/torture.c b/source3/torture/torture.c index 962d0e7..ad30d3d 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -8916,6 +8916,7 @@ static struct { { "SMB2-SESSION-REAUTH", run_smb2_session_reauth }, { "CLEANUP1", run_cleanup1 }, { "CLEANUP2", run_cleanup2 }, + { "CLEANUP3", run_cleanup3 }, { "LOCAL-SUBSTITUTE", run_local_substitute, 0}, { "LOCAL-GENCACHE", run_local_gencache, 0}, { "LOCAL-TALLOC-DICT", run_local_talloc_dict, 0}, diff --git a/source3/wscript_build b/source3/wscript_build index 4deb556..ff1e778 100755 --- a/source3/wscript_build +++ b/source3/wscript_build @@ -1381,6 +1381,7 @@ bld.SAMBA3_BINARY('smbtorture' + bld.env.suffix3, TLDAP -- Samba Shared Repository