Fenrir fixes 2026 09 07 - #169
Conversation
A router that cannot relay a DF-set datagram exceeding the egress link's IP MTU must answer the source with type 3 code 4 carrying the egress MTU (RFC 1812 4.3.2.4); the forward path silently dropped it. The reply goes out the ingress interface to the datagram's source.
sendto() persisted the explicit destination into ts->dst_port/remote_ip before validation, permanently re-pointing a connected socket's peer and RX filter. The destination now applies to the queued datagram only (pinned via ip_output_add_header's src/dst); unconnected sockets keep the last-destination behaviour DHCP/DNS rely on; connect() owns the peer.
A full TX FIFO in CLOSE_WAIT reported writable while send() returns -WOLFIP_EAGAIN, so callers polling readiness busy-loop. can_write now routes CLOSE_WAIT through tx_has_writable_space() like ESTABLISHED (the two states where send accepts data); all other states keep fixed readiness.
…ces it wolfIP_vlan_create reuses the freed slot, so a surviving route, socket, or multicast membership on the deleted index would operate through a newly created VLAN (wrong-VLAN traffic and membership reports). Deletion now returns -WOLFIP_EBUSY until those dependencies are released.
ip_recv marked every directed broadcast local before route selection, so a broadcast for one attached network received on another interface was dropped. A directed broadcast now takes the forward path when a non-ingress egress exists for its network; limited broadcasts and ingress-network broadcasts stay local (no loop).
There was a problem hiding this comment.
🟡 Changes recommended
VLAN deletion still misses some interface-indexed dependencies (e.g., ICMP/raw/packet sockets), which can allow unsafe slot reuse despite the new -WOLFIP_EBUSY guard.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR delivers a set of correctness fixes in the wolfIP stack around forwarding/ICMP behavior, VLAN lifecycle safety, and socket send/write semantics, with accompanying unit-test updates to lock in the regressions.
Changes:
- Add ICMP “Fragmentation Needed” generation for DF-set forwarded packets that exceed egress MTU, and adjust directed-broadcast forwarding behavior.
- Prevent UDP
sendto()from mutating a connected socket’s peer state; adjustip_output_add_header()to accept explicit src/dst addresses. - Reject VLAN deletion while interface-indexed state still references the VLAN slot; update unit tests accordingly.
File summaries
| File | Description |
|---|---|
| wolfip.h | Adds WOLFIP_EBUSY portability macro for new busy-return paths. |
| src/wolfip.c | Implements forwarding fixes (directed broadcast + DF oversize ICMP), adjusts UDP sendto() peer handling, extends TCP can_write, and adds VLAN delete “busy” checks. |
| src/test/unit/unit.c | Registers new/renamed unit tests covering the new behaviors. |
| src/test/unit/unit_tests_vlan.c | Adds regression tests for VLAN delete rejection with dependent state. |
| src/test/unit/unit_tests_socket_api_arms.c | Adds test ensuring can_write reflects TX capacity in CLOSE_WAIT. |
| src/test/unit/unit_tests_proto.c | Updates tests for ip_output_add_header() signature change. |
| src/test/unit/unit_tests_ip_arp_recv.c | Adds forwarding regression tests for DF oversize, directed broadcast forwarding behavior, and related ICMP expectations. |
| src/test/unit/unit_tests_api.c | Updates UDP API tests to validate per-datagram destination encoding and connected-peer preservation. |
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #169
Scan targets checked: wolfip-bugs, wolfip-src
Findings: 4
4 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Reported findings require changes before merge.
VLAN delete now also rejects when a raw socket (WOLFIP_RAWSOCKETS) references the interface. UDP sendto assigns local_ip/if_idx when unset so an unbound socket still accepts replies and getsockname is sane. Directed broadcasts are local-only again: relaying is a smurf vector (RFC 2644 default-deny), local wildcard sockets still receive them.
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #169
Scan targets checked: wolfip-bugs, wolfip-src
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Reported findings require changes before merge.
- wolfIP_send_frag_needed: drop the reply instead of clamping when the original IP total length is smaller than its header (avoids a past-end quote read on a malformed datagram). - esp_send on the frag-needed path uses the VLAN parent's send when the ingress is a VLAN sub-iface (sub-iface send is NULL). - UDP sendto: always set ts->if_idx (the 0-based unset test treated a socket bound to iface 0 as unbound). - Reword the next-hop MTU comment to IP MTU (datagram size, not payload).
The (16) fallback is the POSIX value, used when no system EBUSY is defined; it is not guaranteed to match a non-POSIX libc.
wolfIP_ip_mtu caps at IP_MTU_MAX (1500) but the link carries more (1536-byte frames -> 1522). The frag-needed check and the advertised next-hop MTU now use wolfIP_frame_mtu - ETH_HEADER_LEN so 1501-1522 byte DF-set datagrams forward instead of being black-holed.
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #169
Scan targets checked: wolfip-bugs, wolfip-src
Fenrir result: Approved ✅
No new issues found in the changed files.
Advisory only — this automated result does not count as a GitHub approval.
Fenrir's latest completed scan found no issues; clearing the prior automated change request.
ec9c9b9 F-13187: forward directed broadcasts to attached egress networks
ebfee67 F-13166: reject VLAN deletion while interface-dependent state references it
8d704c2 F-13165: make can_write reflect TX capacity in CLOSE_WAIT
8030a5b F-13164: stop UDP sendto from stealing a connected socket's peer
d0bb26f F-13186: send ICMP Fragmentation Needed for oversized DF-set datagram