[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20180130212213.gvskfv3afboqp4uy@gmail.com>
Date: Tue, 30 Jan 2018 13:22:13 -0800
From: Eric Biggers <ebiggers3@...il.com>
To: Cong Wang <xiyou.wangcong@...il.com>
Cc: Steffen Klassert <steffen.klassert@...unet.com>,
syzbot
<bot+427f0a9138719ba183c0d37d8c2d070567f7761a@...kaller.appspotmail.com>,
David Miller <davem@...emloft.net>,
Herbert Xu <herbert@...dor.apana.org.au>,
LKML <linux-kernel@...r.kernel.org>,
Linux Kernel Network Developers <netdev@...r.kernel.org>,
syzkaller-bugs@...glegroups.com
Subject: Re: WARNING in xfrm_state_fini
On Mon, Nov 27, 2017 at 09:37:07AM -0800, Cong Wang wrote:
> On Mon, Nov 27, 2017 at 3:55 AM, Steffen Klassert
> <steffen.klassert@...unet.com> wrote:
> > On Tue, Nov 21, 2017 at 06:44:04PM -0800, Cong Wang wrote:
> >> User-space uses proto==0 as a wildcard, but xfrm_id_proto_match()
> >> doesn't consider it as a match with IPSEC_PROTO_ANY, in this case
> >> it should match all. Not sure if the following patch is the best way to
> >> fix it, or perhaps x->id.proto should be initialized to some of these 3
> >> values, but looking into ->init_temprop() it is not the case.
> >
> > x->id is copied from the policy template and it seems that we don't
> > validate the id of the template when inserting the policy. iproute2
> > checks for a valid IPsec proto but the kernel does not do so. I think
> > we should check the policy template and reject inserting if the proto
> > is invalid.
> >
>
> Oh, I thought 0 is used as wildcard, so it is not.
>
> Something like below?
>
> @@ -1445,6 +1446,15 @@ static int validate_tmpl(int nr, struct
> xfrm_user_tmpl *ut, u16 family)
> default:
> return -EINVAL;
> }
> + switch (ut[i].id.proto) {
> + case IPPROTO_AH:
> + case IPPROTO_ESP:
> + case IPPROTO_COMP:
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> }
>
> return 0;
>
I assume this is supposed to be fixed by the following, so marking it closed for
syzbot:
#syz fix: xfrm: check id proto in validate_tmpl()
But syzbot has been hitting a WARN_ON() in xfrm_state_fini() even after that
fix, so it should get reported as a new bug.
Powered by blists - more mailing lists