s3:rpc_server: correctly allow up to 65536 workers processes
authorStefan Metzmacher <metze@samba.org>
Thu, 19 Jan 2023 11:27:20 +0000 (12:27 +0100)
committerAndrew Bartlett <abartlet@samba.org>
Tue, 17 Oct 2023 19:20:38 +0000 (19:20 +0000)
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 <metze@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
source3/librpc/idl/rpc_host.idl
source3/rpc_server/rpc_host.c
source3/rpc_server/rpc_worker.c

index 355ef3cc92587118e6bfb6f7a4e95a2cefbd5bc7..129d9227a277d4e3241cc370e839362cefd78f33 100644 (file)
@@ -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;
 
index 432381afb131efad8d20b8835992a5153d958b6b..fcc0d4259d748dd6713e149994429251d310eda0 100644 (file)
@@ -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,
index c41e72381fbf70888cc19d74341e2dd60fadf134..df5298427a040c3294c621eb52dd4324293c5228 100644 (file)
@@ -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);