netdev - Re: [(RFC) PATCH ] NULL pointer dereference on rmmod iptable

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 linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200615114412.GD16460@breakpoint.cc>
Date:   Mon, 15 Jun 2020 13:44:12 +0200
From:   Florian Westphal <fw@...len.de>
To:     Florian Westphal <fw@...len.de>
Cc:     dwilder <dwilder@...ibm.com>, netdev@...r.kernel.org,
        netfilter-devel@...r.kernel.org, wilder@...ibm.com,
        mkubecek@...e.com
Subject: Re: [(RFC) PATCH ] NULL pointer dereference on rmmod iptable_mangle.

Florian Westphal <fw@...len.de> wrote:
> dwilder <dwilder@...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.