Fix mbuf leak in NDP
ClosedPublic
Actions

Authored by smalukav_gmail.com on May 31 2022, 3:14 PM.

Details

Reviewers

melifaro

Group Reviewers

network

Commits

rGa38e2ff92458: netinet6: Fix mbuf leak in NDP
rGd18b4bec98f1: netinet6: Fix mbuf leak in NDP

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

smalukav_gmail.com created this revision.May 31 2022, 3:14 PM

Herald added subscribers: glebius, melifaro, ae, imp. · View Herald TranscriptMay 31 2022, 3:14 PM

smalukav_gmail.com requested review of this revision.May 31 2022, 3:14 PM

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