From a495b15cc4665da36c917d1e04d60644b3989954 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 12 Jun 2026 13:29:12 +0000 Subject: [PATCH 1/2] fix(port): cap queue item size at the slot data bound MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit nxt_port_queue_recv() reads qi->size out of shared memory and uses it verbatim as the memcpy length into a caller-provided buffer sized for NXT_PORT_QUEUE_MSG_SIZE. qi->size is uint8_t, so a peer that can write to the queue (any worker, in the standard threat model for a multi-tenant app server) can set qi->size up to 255 while qi->data is 31 bytes — yielding up to a 224-byte write past the receive buffer. On the main process path this receive buffer lives on the stack of the privileged process. Cap qi->size at NXT_PORT_QUEUE_MSG_SIZE before the memcpy and read into a local first so a concurrent peer write cannot make the bound-check and the memcpy disagree (read-once invariant on shared-memory inputs). Also bound size on nxt_port_queue_send() so a buggy caller that exceeds the per-item budget cannot overwrite the adjacent queue slot. All in-tree callers already pass valid sizes; this is defensive. No behavioural change on the well-formed path. --- src/nxt_port_queue.h | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/src/nxt_port_queue.h b/src/nxt_port_queue.h index d2b2326bf..655ddfa64 100644 --- a/src/nxt_port_queue.h +++ b/src/nxt_port_queue.h @@ -53,6 +53,16 @@ nxt_port_queue_send(nxt_port_queue_t volatile *q, const void *p, uint8_t size, nxt_nncq_atomic_t i; nxt_port_queue_item_t *qi; + /* + * qi->data is NXT_PORT_QUEUE_MSG_SIZE bytes; refuse to enqueue an + * over-sized item rather than overwriting the adjacent queue slot. + * Callers must respect the queue's per-item budget. + */ + if (nxt_slow_path(size > NXT_PORT_QUEUE_MSG_SIZE)) { + *notify = 0; + return NXT_ERROR; + } + i = nxt_nncq_dequeue(&q->free_items); if (i == nxt_nncq_empty(&q->free_items)) { *notify = 0; @@ -77,7 +87,7 @@ nxt_port_queue_send(nxt_port_queue_t volatile *q, const void *p, uint8_t size, nxt_inline ssize_t nxt_port_queue_recv(nxt_port_queue_t volatile *q, void *p) { - ssize_t res; + size_t size; nxt_nncq_atomic_t i; nxt_port_queue_item_t *qi; @@ -88,14 +98,26 @@ nxt_port_queue_recv(nxt_port_queue_t volatile *q, void *p) qi = (nxt_port_queue_item_t *) &q->items[i]; - res = qi->size; - nxt_memcpy(p, qi->data, qi->size); + /* + * qi lives in shared memory that the peer can write. Cap qi->size at + * the slot's data bound (NXT_PORT_QUEUE_MSG_SIZE) before the memcpy so + * a poisoned item cannot overflow the caller's receive buffer, which + * is sized for NXT_PORT_QUEUE_MSG_SIZE. Read into a local first so a + * concurrent peer write cannot make the bound-check and the memcpy + * disagree. + */ + size = qi->size; + if (nxt_slow_path(size > NXT_PORT_QUEUE_MSG_SIZE)) { + size = NXT_PORT_QUEUE_MSG_SIZE; + } + + nxt_memcpy(p, qi->data, size); nxt_nncq_enqueue(&q->free_items, i); nxt_atomic_fetch_add(&q->nitems, -1); - return res; + return size; } From 141773b03747c377401fdc6563e0040bd7fab01b Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 12 Jun 2026 14:47:50 +0000 Subject: [PATCH 2/2] fixup(port): use volatile read for shared-memory qi->size Per Gemini review on PR #27: a non-volatile read of qi->size lets the compiler legally re-load the field from shared memory when generating the memcpy length, defeating the read-once invariant the cap relies on. A concurrent peer write between the bound-check and the second load would bypass the cap and put the stack overflow back. Force the read through a volatile cast so the value is materialised into the local once and the subsequent comparison + memcpy length use that materialised value. --- src/nxt_port_queue.h | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/nxt_port_queue.h b/src/nxt_port_queue.h index 655ddfa64..2eba8c583 100644 --- a/src/nxt_port_queue.h +++ b/src/nxt_port_queue.h @@ -102,11 +102,12 @@ nxt_port_queue_recv(nxt_port_queue_t volatile *q, void *p) * qi lives in shared memory that the peer can write. Cap qi->size at * the slot's data bound (NXT_PORT_QUEUE_MSG_SIZE) before the memcpy so * a poisoned item cannot overflow the caller's receive buffer, which - * is sized for NXT_PORT_QUEUE_MSG_SIZE. Read into a local first so a - * concurrent peer write cannot make the bound-check and the memcpy - * disagree. + * is sized for NXT_PORT_QUEUE_MSG_SIZE. Read through a volatile cast + * so the compiler cannot fold the bound-check and the memcpy length + * into two separate reads of qi->size that a concurrent peer write + * could make disagree. */ - size = qi->size; + size = ((volatile nxt_port_queue_item_t *) qi)->size; if (nxt_slow_path(size > NXT_PORT_QUEUE_MSG_SIZE)) { size = NXT_PORT_QUEUE_MSG_SIZE; }