From 4438addbbcb4caaa92ee165a5e3785853389a755 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Tue, 21 Apr 2020 14:16:41 +0200 Subject: [PATCH 1/6] srvsvc: Introduce ctx3 helper var in enum_file_fn() Bug: https://bugzilla.samba.org/show_bug.cgi?id=14355 Signed-off-by: Volker Lendecke --- source3/rpc_server/srvsvc/srv_srvsvc_nt.c | 25 +++++++++++++---------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c index b5908213b01..c4a8c4b1210 100644 --- a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c +++ b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c @@ -88,9 +88,9 @@ static int enum_file_fn(struct file_id id, { struct file_enum_count *fenum = (struct file_enum_count *)private_data; - + struct srvsvc_NetFileCtr3 *ctr3 = fenum->ctr3; struct srvsvc_NetFileInfo3 *f; - int i = fenum->ctr3->count; + int i = ctr3->count; files_struct fsp; struct byte_range_lock *brl; int num_locks = 0; @@ -111,13 +111,16 @@ static int enum_file_fn(struct file_id id, return 0; } - f = talloc_realloc(fenum->ctx, fenum->ctr3->array, - struct srvsvc_NetFileInfo3, i+1); + f = talloc_realloc( + fenum->ctx, + ctr3->array, + struct srvsvc_NetFileInfo3, + i+1); if ( !f ) { DEBUG(0,("conn_enum_fn: realloc failed for %d items\n", i+1)); return 0; } - fenum->ctr3->array = f; + ctr3->array = f; /* need to count the number of locks on a file */ @@ -152,14 +155,14 @@ static int enum_file_fn(struct file_id id, /* now fill in the srvsvc_NetFileInfo3 struct */ - fenum->ctr3->array[i].fid = + ctr3->array[i].fid = (((uint32_t)(procid_to_pid(&e->pid))<<16) | e->share_file_id); - fenum->ctr3->array[i].permissions = permissions; - fenum->ctr3->array[i].num_locks = num_locks; - fenum->ctr3->array[i].path = fullpath; - fenum->ctr3->array[i].user = username; + ctr3->array[i].permissions = permissions; + ctr3->array[i].num_locks = num_locks; + ctr3->array[i].path = fullpath; + ctr3->array[i].user = username; - fenum->ctr3->count++; + ctr3->count++; return 0; } -- 2.20.1 From 0f9735d43f2d3dc3689f500150fb7144b8e98b6a Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Tue, 21 Apr 2020 14:21:49 +0200 Subject: [PATCH 2/6] srvsvc: Use a struct assignment in enum_file_fn() Looks nicer than 5 complex array references... Bug: https://bugzilla.samba.org/show_bug.cgi?id=14355 Signed-off-by: Volker Lendecke --- source3/rpc_server/srvsvc/srv_srvsvc_nt.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c index c4a8c4b1210..02b676f0fd9 100644 --- a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c +++ b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c @@ -155,12 +155,14 @@ static int enum_file_fn(struct file_id id, /* now fill in the srvsvc_NetFileInfo3 struct */ - ctr3->array[i].fid = - (((uint32_t)(procid_to_pid(&e->pid))<<16) | e->share_file_id); - ctr3->array[i].permissions = permissions; - ctr3->array[i].num_locks = num_locks; - ctr3->array[i].path = fullpath; - ctr3->array[i].user = username; + ctr3->array[i] = (struct srvsvc_NetFileInfo3) { + .fid = (((uint32_t)(procid_to_pid(&e->pid))<<16) | + e->share_file_id), + .permissions = permissions, + .num_locks = num_locks, + .path = fullpath, + .user = username, + }; ctr3->count++; -- 2.20.1 From 7ca9b3b114465d8db8cffff87624b6c798608d17 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Tue, 21 Apr 2020 14:24:48 +0200 Subject: [PATCH 3/6] srvsvc: Directly use "ctr3->count" instead of "i" To me this was not very transparent, and now that we have "ctr3" a single indirect looks okay Bug: https://bugzilla.samba.org/show_bug.cgi?id=14355 Signed-off-by: Volker Lendecke --- source3/rpc_server/srvsvc/srv_srvsvc_nt.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c index 02b676f0fd9..0101325c2e7 100644 --- a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c +++ b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c @@ -90,7 +90,6 @@ static int enum_file_fn(struct file_id id, (struct file_enum_count *)private_data; struct srvsvc_NetFileCtr3 *ctr3 = fenum->ctr3; struct srvsvc_NetFileInfo3 *f; - int i = ctr3->count; files_struct fsp; struct byte_range_lock *brl; int num_locks = 0; @@ -115,9 +114,9 @@ static int enum_file_fn(struct file_id id, fenum->ctx, ctr3->array, struct srvsvc_NetFileInfo3, - i+1); + ctr3->count+1); if ( !f ) { - DEBUG(0,("conn_enum_fn: realloc failed for %d items\n", i+1)); + DBG_ERR("realloc failed for %"PRIu32" items\n", ctr3->count+1); return 0; } ctr3->array = f; @@ -155,7 +154,7 @@ static int enum_file_fn(struct file_id id, /* now fill in the srvsvc_NetFileInfo3 struct */ - ctr3->array[i] = (struct srvsvc_NetFileInfo3) { + ctr3->array[ctr3->count] = (struct srvsvc_NetFileInfo3) { .fid = (((uint32_t)(procid_to_pid(&e->pid))<<16) | e->share_file_id), .permissions = permissions, -- 2.20.1 From bc61d7e8f07628b506a0f5dda7411cd09659ba90 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Tue, 21 Apr 2020 14:42:50 +0200 Subject: [PATCH 4/6] srvsvc: Use a struct initializer in net_enum_files() Bug: https://bugzilla.samba.org/show_bug.cgi?id=14355 Signed-off-by: Volker Lendecke --- source3/rpc_server/srvsvc/srv_srvsvc_nt.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c index 0101325c2e7..e749125d111 100644 --- a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c +++ b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c @@ -176,11 +176,9 @@ static WERROR net_enum_files(TALLOC_CTX *ctx, struct srvsvc_NetFileCtr3 **ctr3, uint32_t resume) { - struct file_enum_count f_enum_cnt; - - f_enum_cnt.ctx = ctx; - f_enum_cnt.username = username; - f_enum_cnt.ctr3 = *ctr3; + struct file_enum_count f_enum_cnt = { + .ctx = ctx, .username = username, .ctr3 = *ctr3 + }; share_entry_forall(enum_file_fn, (void *)&f_enum_cnt ); -- 2.20.1 From 527dae2081658aefafaca84528dc59b4d0a29ae6 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Tue, 21 Apr 2020 14:32:16 +0200 Subject: [PATCH 5/6] srvsvc: Collect file ids in enum_file_fn() Bug: https://bugzilla.samba.org/show_bug.cgi?id=14355 Signed-off-by: Volker Lendecke --- source3/rpc_server/srvsvc/srv_srvsvc_nt.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c index e749125d111..b2f97420dc6 100644 --- a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c +++ b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c @@ -54,6 +54,7 @@ struct file_enum_count { TALLOC_CTX *ctx; const char *username; struct srvsvc_NetFileCtr3 *ctr3; + struct file_id *fids; }; struct sess_file_info { @@ -90,6 +91,7 @@ static int enum_file_fn(struct file_id id, (struct file_enum_count *)private_data; struct srvsvc_NetFileCtr3 *ctr3 = fenum->ctr3; struct srvsvc_NetFileInfo3 *f; + struct file_id *fids = NULL; files_struct fsp; struct byte_range_lock *brl; int num_locks = 0; @@ -121,10 +123,18 @@ static int enum_file_fn(struct file_id id, } ctr3->array = f; + fids = talloc_realloc( + fenum->ctx, fenum->fids, struct file_id, ctr3->count+1); + if (fids == NULL) { + DBG_ERR("realloc failed for %"PRIu32" items\n", ctr3->count+1); + return 0; + } + fids[ctr3->count] = id; + fenum->fids = fids; + /* need to count the number of locks on a file */ - ZERO_STRUCT( fsp ); - fsp.file_id = id; + fsp = (struct files_struct) { .file_id = id }; if ( (brl = brl_get_locks(talloc_tos(), &fsp)) != NULL ) { num_locks = brl_num_locks(brl); -- 2.20.1 From 870b6ceb1fc002bac031f9edf663da366f05ef2e Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Tue, 21 Apr 2020 14:54:25 +0200 Subject: [PATCH 6/6] srvsvc: Move brl_get_locks() out of enum_file_fn() With share_infos.tdb this is a locking order violation: share_infos.tdb is level 4, brlock.tdb is level 2. Avoid this by first walking the share_infos.tdb and then fetching all the brlock entries. Bug: https://bugzilla.samba.org/show_bug.cgi?id=14355 Signed-off-by: Volker Lendecke --- source3/rpc_server/srvsvc/srv_srvsvc_nt.c | 30 +++++++++++++---------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c index b2f97420dc6..03830c515e9 100644 --- a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c +++ b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c @@ -92,9 +92,6 @@ static int enum_file_fn(struct file_id id, struct srvsvc_NetFileCtr3 *ctr3 = fenum->ctr3; struct srvsvc_NetFileInfo3 *f; struct file_id *fids = NULL; - files_struct fsp; - struct byte_range_lock *brl; - int num_locks = 0; char *fullpath = NULL; uint32_t permissions; const char *username; @@ -132,15 +129,6 @@ static int enum_file_fn(struct file_id id, fids[ctr3->count] = id; fenum->fids = fids; - /* need to count the number of locks on a file */ - - fsp = (struct files_struct) { .file_id = id }; - - if ( (brl = brl_get_locks(talloc_tos(), &fsp)) != NULL ) { - num_locks = brl_num_locks(brl); - TALLOC_FREE(brl); - } - if ( strcmp(d->base_name, "." ) == 0 ) { fullpath = talloc_asprintf( fenum->ctx, @@ -168,7 +156,6 @@ static int enum_file_fn(struct file_id id, .fid = (((uint32_t)(procid_to_pid(&e->pid))<<16) | e->share_file_id), .permissions = permissions, - .num_locks = num_locks, .path = fullpath, .user = username, }; @@ -189,11 +176,28 @@ static WERROR net_enum_files(TALLOC_CTX *ctx, struct file_enum_count f_enum_cnt = { .ctx = ctx, .username = username, .ctr3 = *ctr3 }; + uint32_t i; share_entry_forall(enum_file_fn, (void *)&f_enum_cnt ); *ctr3 = f_enum_cnt.ctr3; + /* need to count the number of locks on a file */ + + for (i=0; i<(*ctr3)->count; i++) { + struct files_struct fsp = { .file_id = f_enum_cnt.fids[i] }; + struct byte_range_lock *brl = NULL; + + brl = brl_get_locks(ctx, &fsp); + if (brl == NULL) { + continue; + } + + (*ctr3)->array[i].num_locks = brl_num_locks(brl); + + TALLOC_FREE(brl); + } + return WERR_OK; } -- 2.20.1