RE: [(RFC) PATCH ] NULL pointer dereference on rmmod iptable_mangle.

[dwilder <dwilder@us.ibm.com>]

On 2020-06-15 04:44, Florian Westphal wrote:
> Florian Westphal <fw@strlen.de> wrote:
>> dwilder <dwilder@us.ibm.com> wrote:
>> > > Since the netns core already does an unconditional synchronize_rcu after
>> > > the pre_exit hooks this would avoid the problem as well.
>> >
>> > Something like this?  (un-tested)
>> 
>> Yes.
>> 
>> > diff --git a/net/ipv4/netfilter/iptable_mangle.c
>> > b/net/ipv4/netfilter/iptable_mangle.c
>> > index bb9266ea3785..0d448e4d5213 100644
>> > --- a/net/ipv4/netfilter/iptable_mangle.c
>> > +++ b/net/ipv4/netfilter/iptable_mangle.c
>> > @@ -100,15 +100,26 @@ static int __net_init iptable_mangle_table_init(struct
>> > net *net)
>> >         return ret;
>> >  }
>> >
>> > +static void __net_exit iptable_mangle_net_pre_exit(struct net *net)
>> > +{
>> > +       struct xt_table *table = net->ipv4.iptable_mangle;
>> > +
>> > +       if (mangle_ops)
>> > +               nf_unregister_net_hooks(net, mangle_ops,
>> > +                       hweight32(table->valid_hooks));
>> > +}
>> 
>> You probably need if (table) here, not mangle_ops.
>> I'm not sure if it makes sense to add a new
>> 
>> xt_unregister_table_hook() helper, I guess one would have to try
>> and see if that reduces copy&paste programming or not.
>> 
>> >  static void __net_exit iptable_mangle_net_exit(struct net *net)
>> >  {
>> >         if (!net->ipv4.iptable_mangle)
>> >                 return;
>> > -       ipt_unregister_table(net, net->ipv4.iptable_mangle, mangle_ops);
>> > +       ipt_unregister_table(net, net->ipv4.iptable_mangle, NULL);
>> 
>> I guess the 3rd arg could be removed from the helper.
>> 
>> But yes, this looks like what I had in mind.
> 
> Will there be a followup?  Otherwise I will place this on my todo-list.
> 
> Thanks.

Hi Florian

I am working on a patch.  Will post soon.  Sorry for the delay.