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 for Android: free password hash cracker in your pocket

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

Date:   Mon, 15 Jun 2020 09:21:12 -0700
From:   dwilder <dwilder@...ibm.com>
To:     Florian Westphal <fw@...len.de>
Cc:     netdev@...r.kernel.org, netfilter-devel@...r.kernel.org,
        wilder@...ibm.com, mkubecek@...e.com,
        netfilter-devel-owner@...r.kernel.org
Subject: RE: [(RFC) PATCH ] NULL pointer dereference on rmmod iptable_mangle.

On 2020-06-15 04:44, Florian Westphal wrote:
> 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.

Hi Florian

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