[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#934168: linux-image-4.19.0-5-amd64: iptables-restore may result in NULL pointer dereference at nf_tables_newrule on startup



Hi Elias,

On Thu, Aug 08, 2019 at 12:47:12AM +0200, Elias Werberich wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA512
> 
> Hi Salvatore,
> 
> using the current 5.2.6-1 Debian Kernel fixes this bug.
> I have checked the differences between v4.19 and v5.2 in the upstream kernel
> repository and found the following commit:
> 
> Commit 9b1ef3a0e906bb4a37a71ee39c8528270b490243 from Linux Kernel Upstream:
> > From 9b1ef3a0e906bb4a37a71ee39c8528270b490243 Mon Sep 17 00:00:00 2001
> > From: Taehee Yoo <ap420073@gmail.com>
> > Date: Tue, 19 Mar 2019 13:22:41 +0900
> > Subject: [PATCH] netfilter: nf_tables: add missing ->release_ops() in error
> >  path of newrule()
> > 
> > ->release_ops() callback releases resources and this is used in error path.
> > If nf_tables_newrule() fails after ->select_ops(), it should release
> > resources. but it can not call ->destroy() because that should be called
> > after ->init().
> > At this point, ->release_ops() should be used for releasing resources.
> > 
> > Test commands:
> >    modprobe -rv xt_tcpudp
> >    iptables-nft -I INPUT -m tcp   <-- error command
> >    lsmod
> > 
> > Result:
> >    Module                  Size  Used by
> >    xt_tcpudp              20480  2      <-- it should be 0
> > 
> > Fixes: b8e204006340 ("netfilter: nft_compat: use .release_ops and remove list of extension")
> > Signed-off-by: Taehee Yoo <ap420073@gmail.com>
> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> > ---
> >  net/netfilter/nf_tables_api.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> > index 2cfb173cd0b2..4e57d90f8884 100644
> > --- a/net/netfilter/nf_tables_api.c
> > +++ b/net/netfilter/nf_tables_api.c
> > @@ -2693,8 +2693,11 @@ static int nf_tables_newrule(struct net *net, struct sock *nlsk,
> >  	nf_tables_rule_release(&ctx, rule);
> >  err1:
> >  	for (i = 0; i < n; i++) {
> > -		if (info[i].ops != NULL)
> > +		if (info[i].ops) {
> >  			module_put(info[i].ops->type->owner);
> > +			if (info[i].ops->type->release_ops)
> > +				info[i].ops->type->release_ops(info[i].ops);
> > +		}
> >  	}
> >  	kvfree(info);
> >  	return err;
> > -- 
> > 2.22.0
> 
> AFAIK, this is not backported to Debian Linux Kernel for Buster.
> It would be great if anyone can check if this is the correct commit.

This looks right, and the commit will actually be included when we
rebase to 4.19.67 for buster.

If it is possible for you, please try to do the simple patching (cf.
https://kernel-team.pages.debian.net/kernel-handbook/ch-common-tasks.html#s4.2.2)
and confirm it fixes the issue indedd that would be great. Otherwise I
will try to have a look tomorrow.

Regards,
Salvatore


Reply to: