Messages in this thread | | | Date | Wed, 22 Feb 2023 10:11:03 +0800 | Subject | Re: [PATCH] netfilter: fix percpu counter block leak on error path when creating new netns | From | Pavel Tikhomirov <> |
| |
On 22.02.2023 07:25, Pablo Neira Ayuso wrote: > Hi, > > On Mon, Feb 13, 2023 at 12:25:05PM +0800, Pavel Tikhomirov wrote: >> Here is the stack where we allocate percpu counter block: >> >> +-< __alloc_percpu >> +-< xt_percpu_counter_alloc >> +-< find_check_entry # {arp,ip,ip6}_tables.c >> +-< translate_table >> >> And it can be leaked on this code path: >> >> +-> ip6t_register_table >> +-> translate_table # allocates percpu counter block >> +-> xt_register_table # fails >> >> there is no freeing of the counter block on xt_register_table fail. >> Note: xt_percpu_counter_free should be called to free it like we do in >> do_replace through cleanup_entry helper (or in __ip6t_unregister_table). >> >> Probability of hitting this error path is low AFAICS (xt_register_table >> can only return ENOMEM here, as it is not replacing anything, as we are >> creating new netns, and it is hard to imagine that all previous >> allocations succeeded and after that one in xt_register_table failed). >> But it's worth fixing even the rare leak. > > Any suggestion as Fixes: tag here? This issue seems to be rather old?
If I'm correct:
1) we have this exact percpu leak since commit 71ae0dff02d7 ("netfilter: xtables: use percpu rule counters") which introduced the percpu allocation.
2) but we don't call cleanup_entry on this path at least since commit 1da177e4c3f4 ("Linux-2.6.12-rc2") which is really old.
3) I also see the same thing here https://github.com/mpe/linux-fullhistory/blame/1ab7e5ccf454483fb78998854dddd0bab398c3de/net/ipv4/netfilter/arp_tables.c#L1169 which is probably the initiall commit which introduced net/ipv4/netfilter/arp_tables.c file.
So I'm not sure about Fixes: tag, probably one of those three commits. -- Best regards, Tikhomirov Pavel Software Developer, Virtuozzo.
| |