linux-kernel - Re: WARNING in xfrm_state

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]

Date:   Mon, 27 Nov 2017 09:37:07 -0800
From:   Cong Wang <xiyou.wangcong@...il.com>
To:     Steffen Klassert <steffen.klassert@...unet.com>
Cc:     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 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;