Author: vlendec Date: 2005-08-24 13:15:01 +0000 (Wed, 24 Aug 2005) New Revision: 9584
WebSVN: http://websvn.samba.org/cgi-bin/viewcvs.cgi?view=rev&root=samba&rev=9584 Log: Fix a race condition in Samba 3. If two files are opened simultaneously with NTCREATEX_DISP_CREATE (create if not exists, else fail) they might end up with two or more times NT_STATUS_OK as EEXIST is not correctly handled. Jeremy, please look closely at this. You can easily verify this by adding a smb_msleep(100) to the top of open_file_ntcreate and run the new samba4 torture test. It does also happen without the msleep, but not as reliably. Thanks, Volker Modified: branches/SAMBA_3_0/source/smbd/open.c branches/SAMBA_4_0/source/torture/raw/open.c branches/SAMBA_4_0/source/torture/torture.c trunk/source/smbd/open.c Changeset: Modified: branches/SAMBA_3_0/source/smbd/open.c =================================================================== --- branches/SAMBA_3_0/source/smbd/open.c 2005-08-24 13:09:13 UTC (rev 9583) +++ branches/SAMBA_3_0/source/smbd/open.c 2005-08-24 13:15:01 UTC (rev 9584) @@ -1585,6 +1585,15 @@ fsp_open = open_file(fsp,conn,fname,psbuf,flags|flags2,unx_mode,access_mask); + if (!fsp_open && (flags2 & O_EXCL) && (errno == EEXIST)) { + /* + * Two smbd's tried to open exclusively, but only one of them + * succeeded. + */ + file_free(fsp); + return NULL; + } + if (!fsp_open && (flags == O_RDWR) && (errno != ENOENT)) { if((fsp_open = open_file(fsp,conn,fname,psbuf, O_RDONLY,unx_mode,access_mask)) == True) { Modified: branches/SAMBA_4_0/source/torture/raw/open.c =================================================================== --- branches/SAMBA_4_0/source/torture/raw/open.c 2005-08-24 13:09:13 UTC (rev 9583) +++ branches/SAMBA_4_0/source/torture/raw/open.c 2005-08-24 13:15:01 UTC (rev 9584) @@ -23,6 +23,7 @@ #include "system/time.h" #include "system/filesys.h" #include "librpc/gen_ndr/ndr_security.h" +#include "lib/events/events.h" /* enum for whether reads/writes are possible on a file */ enum rdwr_mode {RDWR_NONE, RDWR_RDONLY, RDWR_WRONLY, RDWR_RDWR}; @@ -1236,7 +1237,131 @@ return ret; } +/* A little torture test to expose a race condition in Samba 3.0.20 ... :-) */ +static BOOL test_raw_open_multi(void) +{ + struct smbcli_state *cli; + TALLOC_CTX *mem_ctx = talloc_init("torture_test_oplock_multi"); + const char *fname = "\\test_oplock.dat"; + NTSTATUS status; + BOOL ret = True; + union smb_open io; + struct smbcli_state **clients; + struct smbcli_request **requests; + union smb_open *ios; + const char *host = lp_parm_string(-1, "torture", "host"); + const char *share = lp_parm_string(-1, "torture", "share"); + int i, num_files = 3; + struct event_context *ev; + int num_ok = 0; + int num_collision = 0; + + ev = event_context_init(mem_ctx); + clients = talloc_array(mem_ctx, struct smbcli_state *, num_files); + requests = talloc_array(mem_ctx, struct smbcli_request *, num_files); + ios = talloc_array(mem_ctx, union smb_open, num_files); + if ((ev == NULL) || (clients == NULL) || (requests == NULL) || + (ios == NULL)) { + DEBUG(0, ("talloc failed\n")); + return False; + } + + if (!torture_open_connection_share(mem_ctx, &cli, host, share, ev)) { + return False; + } + + cli->tree->session->transport->options.request_timeout = 60000; + + for (i=0; i<num_files; i++) { + if (!torture_open_connection_share(mem_ctx, &(clients[i]), + host, share, ev)) { + DEBUG(0, ("Could not open %d'th connection\n", i)); + return False; + } + clients[i]->tree->session->transport-> + options.request_timeout = 60000; + } + + /* cleanup */ + smbcli_unlink(cli->tree, fname); + + /* + base ntcreatex parms + */ + io.generic.level = RAW_OPEN_NTCREATEX; + io.ntcreatex.in.root_fid = 0; + io.ntcreatex.in.access_mask = SEC_RIGHTS_FILE_ALL; + io.ntcreatex.in.alloc_size = 0; + io.ntcreatex.in.file_attr = FILE_ATTRIBUTE_NORMAL; + io.ntcreatex.in.share_access = NTCREATEX_SHARE_ACCESS_READ| + NTCREATEX_SHARE_ACCESS_WRITE| + NTCREATEX_SHARE_ACCESS_DELETE; + io.ntcreatex.in.open_disposition = NTCREATEX_DISP_CREATE; + io.ntcreatex.in.create_options = 0; + io.ntcreatex.in.impersonation = NTCREATEX_IMPERSONATION_ANONYMOUS; + io.ntcreatex.in.security_flags = 0; + io.ntcreatex.in.fname = fname; + io.ntcreatex.in.flags = 0; + + for (i=0; i<num_files; i++) { + ios[i] = io; + requests[i] = smb_raw_open_send(clients[i]->tree, &ios[i]); + if (requests[i] == NULL) { + DEBUG(0, ("could not send %d'th request\n", i)); + return False; + } + } + + DEBUG(10, ("waiting for replies\n")); + while (1) { + BOOL unreplied = False; + for (i=0; i<num_files; i++) { + if (requests[i] == NULL) { + continue; + } + if (requests[i]->state < SMBCLI_REQUEST_DONE) { + unreplied = True; + break; + } + status = smb_raw_open_recv(requests[i], mem_ctx, + &ios[i]); + + DEBUG(0, ("File %d returned status %s\n", i, + nt_errstr(status))); + + if (NT_STATUS_IS_OK(status)) { + num_ok += 1; + } + + if (NT_STATUS_EQUAL(status, + NT_STATUS_OBJECT_NAME_COLLISION)) { + num_collision += 1; + } + + requests[i] = NULL; + } + if (!unreplied) { + break; + } + + if (event_loop_once(ev) != 0) { + DEBUG(0, ("event_loop_once failed\n")); + return False; + } + } + + if ((num_ok != 1) || (num_ok + num_collision != num_files)) { + ret = False; + } + + for (i=0; i<num_files; i++) { + torture_close_connection(clients[i]); + } + talloc_free(mem_ctx); + return ret; +} + /* basic testing of all RAW_OPEN_* calls */ BOOL torture_raw_open(void) @@ -1257,6 +1382,7 @@ ret &= test_ntcreatex_brlocked(cli, mem_ctx); ret &= test_open(cli, mem_ctx); + ret &= test_raw_open_multi(); ret &= test_openx(cli, mem_ctx); ret &= test_ntcreatex(cli, mem_ctx); ret &= test_nttrans_create(cli, mem_ctx); Modified: branches/SAMBA_4_0/source/torture/torture.c =================================================================== --- branches/SAMBA_4_0/source/torture/torture.c 2005-08-24 13:09:13 UTC (rev 9583) +++ branches/SAMBA_4_0/source/torture/torture.c 2005-08-24 13:15:01 UTC (rev 9584) @@ -76,16 +76,17 @@ return NULL; } -BOOL torture_open_connection_share(struct smbcli_state **c, +BOOL torture_open_connection_share(TALLOC_CTX *mem_ctx, + struct smbcli_state **c, const char *hostname, - const char *sharename) + const char *sharename, + struct event_context *ev) { NTSTATUS status; - status = smbcli_full_connection(NULL, - c, hostname, + status = smbcli_full_connection(mem_ctx, c, hostname, sharename, NULL, - cmdline_credentials, NULL); + cmdline_credentials, ev); if (!NT_STATUS_IS_OK(status)) { printf("Failed to open connection - %s\n", nt_errstr(status)); return False; @@ -102,7 +103,7 @@ const char *host = lp_parm_string(-1, "torture", "host"); const char *share = lp_parm_string(-1, "torture", "share"); - return torture_open_connection_share(c, host, share); + return torture_open_connection_share(NULL, c, host, share, NULL); } @@ -2107,9 +2108,11 @@ while (1) { if (hostname) { - if (torture_open_connection_share(¤t_cli, + if (torture_open_connection_share(NULL, + ¤t_cli, hostname, - sharename)) { + sharename, + NULL)) { break; } } else if (torture_open_connection(¤t_cli)) { Modified: trunk/source/smbd/open.c =================================================================== --- trunk/source/smbd/open.c 2005-08-24 13:09:13 UTC (rev 9583) +++ trunk/source/smbd/open.c 2005-08-24 13:15:01 UTC (rev 9584) @@ -1490,6 +1490,15 @@ fsp_open = open_file(fsp,conn,fname,psbuf,flags|flags2,unx_mode,access_mask); + if (!fsp_open && (flags2 & O_EXCL) && (errno == EEXIST)) { + /* + * Two smbd's tried to open exclusively, but only one of them + * succeeded. + */ + file_free(fsp); + return NULL; + } + if (!fsp_open && (flags == O_RDWR) && (errno != ENOENT)) { if((fsp_open = open_file(fsp,conn,fname,psbuf, O_RDONLY,unx_mode,access_mask)) == True) {
