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,
>
