netdev - Re: [PATCH v2] xfrm: correctly check policy index in verify_newpolicy

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] [day] [month] [year] [list]

Message-ID: <5a576347-45e2-edf9-f304-b98057893eb1@huawei.com>
Date:   Wed, 27 Feb 2019 11:17:47 +0800
From:   YueHaibing <yuehaibing@...wei.com>
To:     Herbert Xu <herbert@...dor.apana.org.au>
CC:     <steffen.klassert@...unet.com>, <davem@...emloft.net>,
        <linux-kernel@...r.kernel.org>, <netdev@...r.kernel.org>
Subject: Re: [PATCH v2] xfrm: correctly check policy index in
 verify_newpolicy_info

On 2019/2/25 21:43, Herbert Xu wrote:
> On Mon, Feb 25, 2019 at 05:56:00PM +0800, Yue Haibing wrote:
>>
>> the check. Then __xfrm_policy_unlink use the index to access array policy_count
>> whose size is XFRM_POLICY_MAX * 2, triggering out of bounds access.
> 
> No it doesn't.  Even if it did the bug would be in __xfrm_policy_unlink
> and not here.
> 

Yes, my fix is wrong.

The issue is triggered as this:

xfrm_add_policy
    -->verify_newpolicy_info  //here check the index provided by user with XFRM_POLICY_MAX
			      //In my case, the index is 0x6E6BB6, so it pass the check.
    -->xfrm_policy_construct  //copy the user's policy and set xfrm_policy_timer
    -->xfrm_policy_insert
	--> __xfrm_policy_link //use the orgin dir, in my case is 2
	--> xfrm_gen_index   //generate policy index, there is 0x6E6BB6

then xfrm_policy_timer be fired

xfrm_policy_timer
   --> xfrm_policy_id2dir  //get dir from policy index & 7, in my case is 6
   --> xfrm_policy_delete
      --> __xfrm_policy_unlink //There access policy_count[dir], it trigger out of range access

So maybe the fix is like this:

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 8d1a898..b27eb742 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -316,6 +316,8 @@ static void xfrm_policy_timer(struct timer_list *t)
                goto out;

        dir = xfrm_policy_id2dir(xp->index);
+       if (dir >= XFRM_POLICY_MAX * 2)
+               dir = dir & XFRM_POLICY_MAX;

        if (xp->lft.hard_add_expires_seconds) {
                time64_t tmo = xp->lft.hard_add_expires_seconds +



> Your patch makes no sense.
> 
> Cheers,
>