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

From: Florian Westphal <[email-masked]>
Date: Thu, 4 Jun 2020 12:38:15 +0200

dwilder <[email-masked]> 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.