Page MenuHomeFreeBSD

libpfctl: reset the snl(3) arena per netlink transaction
AbandonedPublic

Authored by oshogbo on Mon, Jun 22, 9:36 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Jul 16, 4:14 AM
Unknown Object (File)
Tue, Jul 14, 10:07 PM
Unknown Object (File)
Sat, Jul 4, 10:02 AM
Unknown Object (File)
Sat, Jul 4, 4:42 AM
Unknown Object (File)
Sat, Jul 4, 4:39 AM
Unknown Object (File)
Fri, Jul 3, 8:11 AM
Unknown Object (File)
Thu, Jul 2, 9:41 AM
Unknown Object (File)
Thu, Jul 2, 6:45 AM
Subscribers

Details

Reviewers
kp
Summary

A pfctl_handle embeds an snl_state whose arena backs both netlink
requests and reply parsing. The arena only grows; it is reset only
by snl_free() in pfctl_close().

Consumers that keep one handle open and poll pf repeatedly -
for example the bsnmpd snmp_pf module - therefore never reset it.

Using DTrace I was able to trace a memory leak:

=== Memory leak, data leaked = 220201152, count = 3
libc.so.7`calloc+0x16e
snmp_pf.so.6`pfctl_get_rule_h+0xe
snmp_pf.so.6`0xea40b321cf3
snmp_pf.so.6`0xea40b3219fb
snmp_pf.so.6`0xea40b321ad9
snmp_pf.so.6`0xea40b3212d9
snmp_pf.so.6`pf_lbltable+0x36
libbsnmp.so.7`snmp_getbulk+0x37f
bsnmpd`snmp_input_finish+0x213
bsnmpd`snmpd_input+0x30e
libbegemot.so.4`poll_dispatch+0x4af
bsnmpd`main+0x9da
libc.so.7`__libc_start1+0x12f
bsnmpd`_start+0x21

After this commit we reset the arena at the start of each transaction,
in a new pfctl_begin_nl() helper.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 74097
Build 70980: arc lint + arc unit

Event Timeline

Oh, thank you somehow I have missed it.

Thanks!

Oh, thank you somehow I have missed it.

Thanks!

Do you have a pointer to the dtrace tool you used? It took me quite a bit of time to track this problem down, because valgrind didn't find it. That's sort of expected, because we still have valid pointers to the memory. (So it's not strictly speaking a leak. We could have called pfctl_close() at any time and released the memory. But functionally it is indeed a leak, of course.)