Florian Westphal <fw@xxxxxxxxx> writes: > Eric W. Biederman <ebiederm@xxxxxxxxxxxx> wrote: >> Florian Westphal <fw@xxxxxxxxx> writes: >> >> > Eric W. Biederman <ebiederm@xxxxxxxxxxxx> 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 -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
