Skip to content

net/udp: fix d_len corruption for 2nd+ SO_REUSEADDR listener#19390

Open
94xhn wants to merge 1 commit into
apache:masterfrom
94xhn:fix-udp-broadcast-dlen-corruption
Open

net/udp: fix d_len corruption for 2nd+ SO_REUSEADDR listener#19390
94xhn wants to merge 1 commit into
apache:masterfrom
94xhn:fix-udp-broadcast-dlen-corruption

Conversation

@94xhn

@94xhn 94xhn commented Jul 9, 2026

Copy link
Copy Markdown

Summary

Fixes #14743. In udp_input()'s broadcast/multicast fan-out loop (net/udp/udp_input.c), each iteration calls netdev_iob_replace(dev, iob) to swap in a freshly cloned iob before handing the packet to the next matching SO_REUSEADDR connection. netdev_iob_replace() unconditionally sets dev->d_len = iob->io_pktlen (net/netdev/netdev_iob.c:149), which is the full frame length (IP + UDP headers + payload) — undoing the dev->d_len -= udpiplen done once before the loop to strip the headers off for udp_input_conn().

As a result, every connection after the first sees a d_len that is udpiplen (IP+UDP header length, e.g. 28 bytes for IPv4) too large. This value flows straight into udp_datahandler() as buflen (net/udp/udp_callback.c:256, int buflen = dev->d_len;) and gets stored as the queued packet's declared length in the connection's read-ahead iob chain. Once more than one such oversized entry has queued up in the same chain, the consumer (udp_readahead() in net/udp/udp_recvfrom.c) parses the following entry's metadata starting at the wrong offset, so whatever byte happens to land on src_addr_size gets trusted as-is. That single byte (0-255) is then used as the length in iob_copyout(srcaddr, iob, src_addr_size, offset) (udp_recvfrom.c:218), which fills a fixed-size stack buffer (uint8_t srcaddr[sizeof(struct sockaddr_in6)]) with no bounds check outside a DEBUGASSERT — compiled out in release builds. An oversized value overflows that stack buffer.

Impact

Affects any target with CONFIG_NET_SOCKOPTS + CONFIG_NET_BROADCAST where two or more SO_REUSEADDR UDP sockets are bound to the same port and receive broadcast/multicast traffic — a normal, non-adversarial configuration (the issue reporter hit it without any malicious input). Security-relevant: stack buffer overflow in the network receive path.

Fix

Re-apply the same -= udpiplen header-stripping after each netdev_iob_replace() call in the loop, matching what's already done once before the loop for the first connection — a 1-line change.

Testing

I don't have a NuttX sim build environment set up in my sandbox (no WSL/Linux host available to me), so I couldn't produce sim:nsh/sim:netnsh execution logs. What I did instead:

  1. Traced the exact value flow through net/udp/udp_input.cnet/netdev/netdev_iob.cnet/udp/udp_callback.cnet/udp/udp_recvfrom.c against current master, confirming every intermediate read/write of d_len/buflen/datalen/src_addr_size cited above by line number.
  2. Extracted just the relevant field-level arithmetic (d_len tracking across the fan-out loop, netdev_iob_replace() copied verbatim) into a standalone host-C reproduction, compiled and run with plain gcc (no NuttX build needed since this part has no OS dependency):
    • Before fix: conn1 sees d_len = 1014 (correct), conn2 and conn3 both see d_len = 1042 — exactly 28 (udpiplen) too large.
    • After fix (-DFIXED, i.e. this patch's one added line): all three connections see the correct d_len = 1014.
  3. Cross-checked against the issue reporter's own finding: they independently added a DEBUGASSERT() to udp_readahead() that fires, and observed "the value of dev->d_len matched the size of the application data before udp_input_conn() was called the 1st time... The second time (for the second [connection])" — the same first-vs-second-connection d_len mismatch my trace and repro reproduce.

Happy to run the real sim:netnsh reproduction and post logs if a maintainer can point me to (or confirm) the right multi-SO_REUSEADDR-socket test setup, or if this needs stronger evidence before merge.

In udp_input()'s broadcast/multicast fan-out loop, each iteration
calls netdev_iob_replace(dev, iob) to swap in a freshly cloned iob
before handing the packet to the next matching connection. That
function unconditionally sets dev->d_len = iob->io_pktlen, which is
the full frame length (IP + UDP headers + payload), undoing the
'dev->d_len -= udpiplen' done once before the loop to strip the
headers off for udp_input_conn().

As a result, every connection after the first sees a d_len that is
udpiplen (IP+UDP header length, eg 28 bytes for IPv4) too large.
This value flows into udp_datahandler() as buflen (it reads
dev->d_len directly) and is stored as the queued packet's declared
length in the connection's read-ahead iob chain. Once more than one
such oversized entry has queued up in the same chain, the consumer
(udp_readahead() in udp_recvfrom.c) parses the following entry's
metadata starting at the wrong offset, so whatever byte happens to
land on src_addr_size is trusted as-is. That single byte (0-255) is
then used as the length in iob_copyout(srcaddr, iob, src_addr_size,
...), which fills a fixed-size stack buffer with no bounds check
outside a DEBUGASSERT - compiled out in release builds - so an
oversized value overflows that stack buffer.

Re-apply the same '-= udpiplen' header-stripping after each
netdev_iob_replace() call in the loop, matching what's already done
once before the loop for the first connection.

Fixes apache#14743

Signed-off-by: yi chen <94xhn1@gmail.com>
@github-actions github-actions Bot added Area: Networking Effects networking subsystem Size: XS The size of the change in this PR is very small labels Jul 10, 2026
@github-actions

Copy link
Copy Markdown

MemBrowse Memory Report

No memory changes detected for:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: Networking Effects networking subsystem Size: XS The size of the change in this PR is very small

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] UDP RX stack corruption (if multiple matching connections)

2 participants