Re: BUG: unable to handle kernel NULL pointer dereference at 00000000000000a0

["Justin P. Mattock" <justinmattock@gmail.com>]

On 06/23/2010 01:34 PM, Timo Teräs wrote:
> On 06/23/2010 09:20 PM, Justin P. Mattock wrote:
>> On 06/23/10 11:10, Timo Teräs wrote:
>>> On 06/23/2010 08:29 PM, Eric Dumazet wrote:
>>>
>>>> Le mercredi 23 juin 2010 à 10:00 -0700, Justin P. Mattock a écrit :
>>>>
>>>>> o.k. the bisect is pointing to the below results..
>>>>> (I tried git revert xxx but this commit is too big
>>>>> so I'll(hopefully)manually revert it on the latest HEAD to
>>>>> see if this is the actual problem im experiencing)
>>>>>
>>>>> 80c802f3073e84c956846e921e8a0b02dfa3755f is the first bad commit
>>>>> commit 80c802f3073e84c956846e921e8a0b02dfa3755f
>>>>> Author: Timo Teräs<timo.teras@iki.fi>
>>>>> Date:   Wed Apr 7 00:30:05 2010 +0000
>>>>>
>>>>>        xfrm: cache bundles instead of policies for outgoing flows
>>>>>
>>>>>        __xfrm_lookup() is called for each packet transmitted out of
>>>>>        system. The xfrm_find_bundle() does a linear search which can
>>>>>        kill system performance depending on how many bundles are
>>>>>        required per policy.
>>>>>
>>>>>        This modifies __xfrm_lookup() to store bundles directly in
>>>>>        the flow cache. If we did not get a hit, we just create a new
>>>>>        bundle instead of doing slow search. This means that we can now
>>>>>        get multiple xfrm_dst's for same flow (on per-cpu basis).
>>>>>
>>>>>        Signed-off-by: Timo Teras<timo.teras@iki.fi>
>>>>>        Signed-off-by: David S. Miller<davem@davemloft.net>
>>>>>
>>>>> :040000 040000 d8e60f5fa4c1329f450d9c7cdf98b34e6a177f22
>>>>> 9f576e68e5bf4ce357d7f0305aee5f410250dfe2 M      include
>>>>> :040000 040000 f2876df688ee36907af7b4123eea96592faaed3e
>>>>> a3f6f6f94f0309106856cd99b38ec90b024eb016 M      net
>>>>>
>>>> Thanks a lot for bisecting Jutin, this is really appreciated.
>>>>
>>>> crash is in xfrm_bundle_ok()
>>>>
>>>> if (xdst->policy_genid != atomic_read(&xdst->pols[0]->genid))
>>>>      return 0;
>>>>
>>>> xdst->pols[0] contains a NULL pointer
>>>>
>>> That does not really make sense, if we get this far; there's a valid
>>> xfrm_state with the bundle. This means that there existed a policy with
>>> it too.
>>>
>>> I'll take a deeper look at this tomorrow. Would it be possible to see
>>> your xfrm policies?
>>>
>>> - Timo
>>>
>>>
>>
>> @Eric sure no problem doing the bisect...
>>
>> as for the xfrm policy here is the link that I used to setup ipsec:
>> http://www.linuxfromscratch.org/hints/downloads/files/ipsec.txt
>> (just change the keys if doing real world work..(but for me just testing).
>>
>> below is a temporary fix for me to get this working, tcpdump reports
>> everything is doing what it should be
>> 11:16:32.496166 IP xxxxx>  xxxxx: AH(spi=0x00000200,seq=0x1090):
>> ESP(spi=0x00000201,seq=0x1090), length 56
>> 11:16:32.496212 IP xxxxx>  xxxxx: AH(spi=0x00000200,seq=0x1091):
>> ESP(spi=0x00000201,seq=0x1091), length 56
>> 11:16:32.496259 IP xxxxx>  xxxxx: AH(spi=0x00000200,seq=0x1092):
>> ESP(spi=0x00000201,seq=0x1092), length 56
>>
>> (tested a few mins ago, but not the right fix..)
>
> Yes, that would break some other obscure scenarios.
>
> Looks like it's ah inside esp. So you get chain of bundles. And only the
> first bundle gets a policy. Should have thought of that. Does the below
> fix it for you?
>
> diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
> index 4bf27d9..af1c173 100644
> --- a/net/xfrm/xfrm_policy.c
> +++ b/net/xfrm/xfrm_policy.c
> @@ -2300,7 +2300,8 @@ int xfrm_bundle_ok(struct xfrm_policy *pol, struct
> xfrm_dst *first,
>   			return 0;
>   		if (xdst->xfrm_genid != dst->xfrm->genid)
>   			return 0;
> -		if (xdst->policy_genid != atomic_read(&xdst->pols[0]->genid))
> +		if (xdst->num_pols>  0&&
> +		    xdst->policy_genid != atomic_read(&xdst->pols[0]->genid))
>   			return 0;
>
>   		if (strict&&  fl&&
>
>


yeah this works.. I can see AH and ESP showing up with tcpdump..
looks good..

Reported-Bisected-By: Justin P. Mattock <justinmattock@gmail.com>

cheers,

Justin P. Mattock