lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sat, 14 May 2016 22:00:09 -0500 From: ebiederm@...ssion.com (Eric W. Biederman) To: Florian Westphal <fw@...len.de> Cc: Pablo Neira Ayuso <pablo@...filter.org>, netfilter-devel@...r.kernel.org, dale.4d@...il.com, netdev@...r.kernel.org Subject: Re: [PATCH nf V2] netfilter: fix oops in nfqueue during netns error unwinding Florian Westphal <fw@...len.de> writes: > Eric W. Biederman <ebiederm@...ssion.com> wrote: >> Florian Westphal <fw@...len.de> writes: >> >> > Eric W. Biederman <ebiederm@...ssion.com> wrote: >> >> Florian could you test and verify this patch fixes your issues? >> > >> > Yes, this seems to work. >> > >> > Pablo, I'm fine with this patch going into -nf/stable but I do not think >> > making the pointers per netns is a desireable option in the long term. >> > >> >> Unlike the other possibilities that have been discussed this also >> >> addresses the nf_queue path as well as the nf_queue_hook_drop path. >> > >> > The nf_queue path should have been fine, no? >> > >> > Or putting it differently: can we start processing skbs before a netns >> > is fully initialized? >> >> The practical case that worries me is what happens when someone does >> "rmmod nfnetlink_queue" while the system is running. It appears to me >> that today we could free the per netns data during the rcu grace period >> and cause a similar issue in nfnl_queue_pernet. >> >> That looks like it could affect both the nf_queue path and the >> nf_queue_nf_hook_drop path. > > OK, I'll check this again but I seem to recall this was fine (the > nfqueue module exit path sets the handler to NULL before doing anything > else). Good point. Yes, the nfnetlink_queue module calls nf_unregister_queue_handler() in the module fini method before it does anything else. That does set queue_handler to NULL and calls synchronize_rcu() before anything else. So in practice that is not a problem, but being disconnected from everything else that is not immediately apparent. Sigh. > The normal netns exit path should be fine too as exit and free happens > in two distinct loops, i.e. while (without your change) we can have > calls to nf_queue_hook_drop after the nfqueue netns exit function was > called, these calls will always happen before the pernets data is > freed. Ouch. That is a little scary. Today we only have remove_proc_entry in nfnl_queue_net_exit. If we had something more substantial those calls after .exit (without my change) we could get into nasty to find oopses. So I guess I did prevent a possible future issue with my patch. I am half wondering if we could make everything simpler by simply not allowing nfnetlink_queue be a module. Eric
Powered by blists - more mailing lists