Skip to content

eigrpd: compute received packet length from actual IP header size#22255

Open
uwezkhan wants to merge 1 commit into
FRRouting:masterfrom
uwezkhan:eigrp-ip-header-len
Open

eigrpd: compute received packet length from actual IP header size#22255
uwezkhan wants to merge 1 commit into
FRRouting:masterfrom
uwezkhan:eigrp-ip-header-len

Conversation

@uwezkhan

@uwezkhan uwezkhan commented Jun 8, 2026

Copy link
Copy Markdown

eigrp_read derives the EIGRP payload length as iph->ip_len - 20U, which assumes the IPv4 header is always 20 bytes. When a received packet carries IP options the real header is iph->ip_hl * 4, so the length is too large by up to 40 bytes. eigrp_read passes that length to the opcode handlers (eigrp_hello_receive, eigrp_update_receive, eigrp_query_receive, eigrp_reply_receive), which walk a raw TLV pointer over the receive buffer bounded only by it, so a packet near the buffer size reads past the end.

Before, the payload length subtracts a fixed 20 regardless of the header, so options shift the parser past the real data. After, it subtracts iph->ip_hl << 2, the same way ospf_read advances over the IP header before parsing. The behavioral difference is limited to packets that actually carry IP options: those now report the correct, shorter payload and the trailing bytes are no longer treated as TLVs.

@greptile-apps

greptile-apps Bot commented Jun 8, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes a one-line bug in eigrpd/eigrp_packet.c where the EIGRP payload length was derived by subtracting a hard-coded 20 from ip_len, ignoring the possibility of IPv4 options extending the header beyond the minimum 20 bytes.

  • Root cause fixed: length = iph->ip_len - 20U is replaced with length = iph->ip_len - (iph->ip_hl << 2), so the actual IP header length is subtracted rather than a constant, exactly matching the pattern already used in ospf_read and in eigrp_recv_packet's own stream_forward_getp call.
  • Impact: Without the fix, a peer sending a packet with IP options would cause length to be up to 40 bytes too large, and the TLV-walking loops in every opcode handler (eigrp_hello_receive, eigrp_update_receive, etc.) would iterate over bytes beyond the real EIGRP payload.

Confidence Score: 5/5

Safe to merge. The change is a minimal, targeted correction to a well-understood off-by-header bug with no new control flow or data paths introduced.

The single-line change replaces a hard-coded constant with the value already computed and used elsewhere in the same function. The fix is consistent with how ospf_read and the stream_forward_getp call in the same function handle the IP header length, and all downstream consumers of length already subtract EIGRP_HEADER_LEN before walking TLVs, so the semantics remain correct.

No files require special attention.

Important Files Changed

Filename Overview
eigrpd/eigrp_packet.c Single-line fix: uses iph->ip_hl << 2 instead of the constant 20U when computing the EIGRP payload length, correctly accounting for IPv4 options. Change is minimal, mirrors ospf_read, and is consistent with how the same function already advances the stream pointer past the IP header.

Sequence Diagram

sequenceDiagram
    participant Kernel as IP Stack
    participant recv as eigrp_recv_packet()
    participant read as eigrp_read()
    participant handler as Opcode Handler

    Kernel->>recv: raw IP packet (may have options)
    recv->>recv: sockopt_iphdrincl_swab_systoh(iph)
    recv->>recv: "validate ret == ip_len"
    recv-->>read: ibuf (stream with IP+EIGRP data)

    read->>read: "iph = STREAM_DATA(ibuf)"
    Note over read: BEFORE: length = ip_len - 20U (wrong when ip_hl > 5)
    Note over read: AFTER: length = ip_len - ip_hl<<2 (correct for any header size)
    read->>read: "validate ip_hl*4 + EIGRP_HEADER_LEN <= endp"
    read->>read: "stream_forward_getp(ibuf, ip_hl*4)"
    read->>read: "eigrph = stream_pnt(ibuf)"
    read->>read: stream_forward_getp(ibuf, EIGRP_HEADER_LEN)
    read->>handler: length passed to handler
    handler->>handler: "size -= EIGRP_HEADER_LEN"
    handler->>handler: "walk TLVs while size >= TLV_HDR_LENGTH"
Loading

Reviews (1): Last reviewed commit: "eigrpd: compute received packet length f..." | Re-trigger Greptile

eigrp_read() assumed a fixed 20-byte IPv4 header when deriving the EIGRP
payload length. That is incorrect whenever IP options are present, since
the header can be longer than 20 bytes. Use the IHL field (ip_hl << 2)
to subtract the actual header length instead.

Signed-off-by: uwezkhan <uwezkhan053@gmail.com>
@uwezkhan uwezkhan force-pushed the eigrp-ip-header-len branch from ea2a737 to d25cadf Compare June 8, 2026 16:37

@riw777 riw777 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

@riw777 riw777 added the bugfix label Jun 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants