Page MenuHomeFreeBSD

Fix mbuf leak in NDP
ClosedPublic

Authored by smalukav_gmail.com on May 31 2022, 3:14 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 1, 7:27 PM
Unknown Object (File)
Sun, Oct 20, 12:11 AM
Unknown Object (File)
Oct 2 2024, 6:07 AM
Unknown Object (File)
Oct 2 2024, 5:56 AM
Unknown Object (File)
Sep 30 2024, 5:49 PM
Unknown Object (File)
Sep 9 2024, 12:12 PM
Unknown Object (File)
Sep 8 2024, 8:25 AM
Unknown Object (File)
Sep 5 2024, 6:52 PM

Details

Summary

Incomplete records may contain packets in their hold queue - https://reviews.freebsd.org/source/src/browse/main/sys/net/if_llatbl.h;89e58b955cf5d2187af5f2460d11b64f4d229c8a$69. When you remove an incomplete record, its packets should be freed in
lltable_drop_entry_queue - https://reviews.freebsd.org/source/src/browse/main/sys/net/if_llatbl.c;89e58b955cf5d2187af5f2460d11b64f4d229c8a$286. But NDP code never increases la_numheld, and the removal doesn't happen.

To fix I make a common function to add a packet to the record's hold queue - lltable_append_entry_queue. I use this function in ARP and NDP code instead of their custom implementations.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

LGTM! Put a number of non-critical comments. Should be good to land once addressed.

sys/net/if_llatbl.c
141

Worth adding WLOCK_ASSERT here

143

Could you declare next here?

151

Could you declare cure here?

322–323
sys/netinet/icmp6.h
605

s/reply/resolution/ ?

sys/netinet/if_ether.c
542–543

I'd really prefer to split actual work and counter increment. If someone decides (for whatever reason) to do something with the counters, including things like define ARPSTAT_ADD, this will break in a weird way.
Could you make it 2 lines?

sys/netinet6/nd6.c
809
2489–2490

Could you split it into 2 lines?

usr.bin/netstat/inet6.c
998
This revision is now accepted and ready to land.May 31 2022, 8:01 PM
This revision was automatically updated to reflect the committed changes.