From: Stefan Metzmacher Date: Thu, 19 Jan 2023 11:27:20 +0000 (+0100) Subject: s3:rpc_server: correctly allow up to 65536 workers processes X-Git-Tag: talloc-2.4.2~1257 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=eb8cf371b8dc9575e2b838ac8e4f03518eb092da;p=thirdparty%2Fsamba.git s3:rpc_server: correctly allow up to 65536 workers processes We already limit the per worker portion of the association group id to UINT16_MAX, so we can also use 16-bit instead of just 8-bit to encode the worker index. While there we should actually ensure that the max worker index is UINT16_MAX. Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett --- diff --git a/source3/librpc/idl/rpc_host.idl b/source3/librpc/idl/rpc_host.idl index 355ef3cc925..129d9227a27 100644 --- a/source3/librpc/idl/rpc_host.idl +++ b/source3/librpc/idl/rpc_host.idl @@ -65,6 +65,10 @@ interface rpc_host_msg /** * @brief Which of the processes of a helper prog is this from + * + * @note while this is uint32, we currently only support 16-bit + * values, as we use it in the high 16-bits of the 32-bit + * association group id. */ uint32 worker_index; diff --git a/source3/rpc_server/rpc_host.c b/source3/rpc_server/rpc_host.c index 432381afb13..fcc0d4259d7 100644 --- a/source3/rpc_server/rpc_host.c +++ b/source3/rpc_server/rpc_host.c @@ -603,6 +603,15 @@ static void rpc_server_get_endpoints_done(struct tevent_req *subreq) tevent_req_error(req, ret); return; } + /* + * We need to limit the number of workers in order + * to put the worker index into a 16-bit space, + * in order to use a 16-bit association group space + * per worker. + */ + if (state->num_workers > 65536) { + state->num_workers = 65536; + } state->idle_seconds = smb_strtoul( lines[1], NULL, 10, &ret, SMB_STR_FULL_STR_CONV); @@ -1235,7 +1244,10 @@ static struct rpc_work_process *rpc_host_find_idle_worker( * All workers are busy. We need to expand the number of * workers because we were asked for an idle worker. */ - if (num_workers+1 < num_workers) { + if (num_workers >= UINT16_MAX) { + /* + * The worker index would not fix into 16-bits + */ return NULL; } tmp = talloc_realloc( @@ -1282,7 +1294,7 @@ again: if (assoc_group_id != 0) { size_t num_workers = talloc_array_length(server->workers); - uint8_t worker_index = assoc_group_id >> 24; + uint16_t worker_index = assoc_group_id >> 16; if (worker_index >= num_workers) { DBG_DEBUG("Invalid assoc group id %"PRIu32"\n", @@ -1292,7 +1304,7 @@ again: worker = &server->workers[worker_index]; if ((worker->pid == -1) || !worker->available) { - DBG_DEBUG("Requested worker index %"PRIu8": " + DBG_DEBUG("Requested worker index %"PRIu16": " "pid=%d, available=%d\n", worker_index, (int)worker->pid, diff --git a/source3/rpc_server/rpc_worker.c b/source3/rpc_server/rpc_worker.c index c41e72381fb..df5298427a0 100644 --- a/source3/rpc_server/rpc_worker.c +++ b/source3/rpc_server/rpc_worker.c @@ -612,7 +612,7 @@ static struct dcesrv_assoc_group *rpc_worker_assoc_group_reference( void *id_ptr = NULL; /* find an association group given a assoc_group_id */ - id_ptr = idr_find(conn->dce_ctx->assoc_groups_idr, id & 0xffffff); + id_ptr = idr_find(conn->dce_ctx->assoc_groups_idr, id & UINT16_MAX); if (id_ptr == NULL) { DBG_NOTICE("Failed to find assoc_group 0x%08x\n", id); return NULL; @@ -647,7 +647,7 @@ static int rpc_worker_assoc_group_destructor( ret = idr_remove( assoc_group->dce_ctx->assoc_groups_idr, - assoc_group->id & 0xffffff); + assoc_group->id & UINT16_MAX); if (ret != 0) { DBG_WARNING("Failed to remove assoc_group 0x%08x\n", assoc_group->id); @@ -659,7 +659,7 @@ static int rpc_worker_assoc_group_destructor( allocate a new association group */ static struct dcesrv_assoc_group *rpc_worker_assoc_group_new( - struct dcesrv_connection *conn, uint8_t worker_index) + struct dcesrv_connection *conn, uint16_t worker_index) { struct dcesrv_context *dce_ctx = conn->dce_ctx; const struct dcesrv_endpoint *endpoint = conn->endpoint; @@ -673,6 +673,11 @@ static struct dcesrv_assoc_group *rpc_worker_assoc_group_new( return NULL; } + /* + * We use 16-bit to encode the worker index, + * have 16-bits left within the worker to form a + * 32-bit association group id. + */ id = idr_get_new_random( dce_ctx->assoc_groups_idr, assoc_group, 1, UINT16_MAX); if (id == -1) { @@ -680,7 +685,7 @@ static struct dcesrv_assoc_group *rpc_worker_assoc_group_new( DBG_WARNING("Out of association groups!\n"); return NULL; } - assoc_group->id = (worker_index << 24) + id; + assoc_group->id = (((uint32_t)worker_index) << 16) | id; assoc_group->transport = transport; assoc_group->dce_ctx = dce_ctx; @@ -698,10 +703,10 @@ static NTSTATUS rpc_worker_assoc_group_find( uint32_t assoc_group_id = call->pkt.u.bind.assoc_group_id; if (assoc_group_id != 0) { - uint8_t worker_index = (assoc_group_id & 0xff000000) >> 24; + uint16_t worker_index = (assoc_group_id & 0xffff0000) >> 16; if (worker_index != w->status.worker_index) { - DBG_DEBUG("Wrong worker id %"PRIu8", " - "expected %"PRIu8"\n", + DBG_DEBUG("Wrong worker id %"PRIu16", " + "expected %"PRIu32"\n", worker_index, w->status.worker_index); return NT_STATUS_NOT_FOUND; @@ -801,7 +806,7 @@ static struct tevent_req *rpc_worker_send( tevent_req_error(req, EINVAL); return tevent_req_post(req, ev); } - if ((worker_index < 0) || ((unsigned)worker_index > UINT32_MAX)) { + if ((worker_index < 0) || ((unsigned)worker_index > UINT16_MAX)) { DBG_ERR("Invalid worker index %d\n", worker_index); tevent_req_error(req, EINVAL); return tevent_req_post(req, ev);