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)
Sat, Jun 29, 3:24 AM
Unknown Object (File)
Thu, Jun 27, 3:14 AM
Unknown Object (File)
May 16 2024, 12:44 AM
Unknown Object (File)
May 15 2024, 12:58 PM
Unknown Object (File)
Apr 26 2024, 3:07 PM
Unknown Object (File)
Apr 8 2024, 7:09 PM
Unknown Object (File)
Jan 29 2024, 6:56 AM
Unknown Object (File)
Jan 16 2024, 6:39 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 Skipped
Unit
Tests Skipped

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
2488–2489

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.