Skip to content

net/igmp: fix checksum validation that always dropped valid IGMP packets - #19720

Merged
xiaoxiang781216 merged 1 commit into
apache:masterfrom
zhekunren:fix/igmp-checksum-validation
Aug 7, 2026
Merged

net/igmp: fix checksum validation that always dropped valid IGMP packets#19720
xiaoxiang781216 merged 1 commit into
apache:masterfrom
zhekunren:fix/igmp-checksum-validation

Conversation

@zhekunren

Copy link
Copy Markdown
Contributor

Summary

igmp_input() rejected every well-formed IGMP packet as a checksum error because the validation compared net_chksum() against 0 instead of 0xffff. This breaks IGMP membership query/report processing entirely whenever CONFIG_NET_IGMP_CHECKSUMS is enabled.

Problem

In net/igmp/igmp_input.c, the IGMP checksum was validated as:

#ifdef CONFIG_NET_IGMP_CHECKSUMS
  if (net_chksum((FAR uint16_t *)igmp, IGMP_HDRLEN) != 0)
    {
      IGMP_STATINCR(g_netstats.igmp.chksum_errors);
      nwarn("WARNING: Checksum error\n");
      goto drop;
    }
#endif

With CONFIG_NET_IGMP_CHECKSUMS enabled, every valid IGMP message hits the goto drop path and is silently discarded, so the host never answers Membership Queries and never processes Membership Reports from neighbors.

Root Cause

net_chksum() (in net/utils/net_chksum.c) returns the raw one's complement sum of all 16-bit words over the buffer — it does not take the one's complement of that sum:

uint16_t net_chksum(FAR uint16_t *data, uint16_t len)
{
  return HTONS(chksum(0, (uint8_t *)data, len));
}

For a valid IGMP packet, the sender (net/igmp/igmp_send.c) stores igmp->chksum = ~igmp_chksum(...). Let S be the one's complement sum of the header with the checksum field zeroed. The received packet therefore contains ~S in the checksum field, and summing all 16-bit words yields:

S + ~S = 0xffff   (one's complement arithmetic)

So the correct validation is != 0xffff, not != 0. The old check != 0 is true for every legal IGMP packet (it can only be 0 in the degenerate case where the sum is exactly 0), so all valid packets were dropped.

This is the same Internet checksum principle used by IPv4 (RFC 1071) and TCP/UDP.

Fix

Compare against 0xffff:

  if (net_chksum((FAR uint16_t *)igmp, IGMP_HDRLEN) != 0xffff)

Consistency

This matches the convention already used by the other transport input handlers in the tree:

  • net/devif/ipv4_input.c(ipv4_chksum(IPv4BUF) != 0xffff)
  • net/tcp/tcp_input.c(tcp_chksum(dev) != 0xffff)

and is symmetric with the sender side in net/igmp/igmp_send.c, which writes igmp->chksum = ~igmp_chksum(...).

Impact

  • Bug fix only, no behavioral change for malformed packets (they still go to goto drop).
  • Restores IGMP receive path when CONFIG_NET_IGMP_CHECKSUMS=y: Membership Queries now correctly trigger the delaying-member state machine, and Membership Reports correctly cancel the report timer.
  • No impact when CONFIG_NET_IGMP_CHECKSUMS is disabled (the block is compiled out).
  • Related to the earlier fix f11df565 ("net/igmp: fix length check that always dropped valid IGMP packets") — together they restore a fully broken IGMP input path.

Testing

Verified by inspection against net_chksum() semantics and cross-checked with ipv4_input.c / tcp_input.c. Code path is gated behind CONFIG_NET_IGMP_CHECKSUMS so the default build is unchanged.

@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 Aug 6, 2026
@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown

MemBrowse Memory Report

No memory changes detected for:

@acassis acassis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@zhekunren if you used AI to help you (based on long messages in the commit message), please include: Assisted-by: the name of AI

igmp_input() validated the IGMP checksum with:

  if (net_chksum((FAR uint16_t *)igmp, IGMP_HDRLEN) != 0)

but net_chksum() returns the raw one's complement sum of the 16-bit
words (it does NOT take the one's complement of that sum). For a valid
IGMP packet whose checksum field holds ~S (as written by igmp_send()),
the sum of all 16-bit words is S + ~S = 0xffff, never 0.

So the existing check `!= 0` was always true for any well-formed IGMP
message, sending every valid packet down the "Checksum error" path to
be silently dropped and breaking IGMP membership query/report processing.

Compare against 0xffff instead, matching the convention used by the
other transport input handlers:

  - ipv4_input.c: (ipv4_chksum(IPv4BUF) != 0xffff)
  - tcp_input.c:  (tcp_chksum(dev) != 0xffff)

This is also consistent with the sender side in igmp_send.c, which
stores `igmp->chksum = ~igmp_chksum(...)`.

Signed-off-by: zhekunren <zhekunren@qq.com>
Assisted-by: GLM-5.2 <noreply@z.ai>
@zhekunren
zhekunren force-pushed the fix/igmp-checksum-validation branch from abc50d8 to a233f9d Compare August 7, 2026 02:02
@zhekunren

zhekunren commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

@zhekunren if you used AI to help you (based on long messages in the commit message), please include: Assisted-by: the name of AI
Ok. Commit tag Assisted‑by: GLM-5.2 noreply@z.ai is added as required.

@xiaoxiang781216
xiaoxiang781216 merged commit 8a0f354 into apache:master Aug 7, 2026
54 checks passed
@zhekunren
zhekunren deleted the fix/igmp-checksum-validation branch August 7, 2026 08:57
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.

3 participants