netdev - Re: [PATCH][IPSEC][4/7] inter address family ipsec tunnel

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]

Message-ID: <456FB283.9090904@miyazawa.org>
Date:	Fri, 01 Dec 2006 13:41:39 +0900
From:	Kazunori MIYAZAWA <kazunori@...azawa.org>
To:	David Miller <davem@...emloft.net>
CC:	miika@....fi, Diego.Beltrami@...t.fi, herbert@...dor.apana.org.au,
	netdev@...r.kernel.org, usagi-core@...ux-ipv6.org
Subject: Re: [PATCH][IPSEC][4/7] inter address family ipsec tunnel

David Miller wrote:
> From: Kazunori MIYAZAWA <kazunori@...azawa.org>
> Date: Fri, 24 Nov 2006 14:38:39 +0900
> 
> What is going on here?
> 
>> +			/* Without this, the atomic inc below segfaults */
>> +			if (encap_family == AF_INET6) {
>> +				rt->peer = NULL;
>> +				rt_bind_peer(rt,1);
>> +			}
>  ...
>> -		dst_prev->output	= xfrm4_output;
>> +		if (dst_prev->xfrm->props.family == AF_INET)
>> +			dst_prev->output = xfrm4_output;
>> +#if defined(CONFIG_IPV6) || defined (CONFIG_IPV6_MODULE)
>> +		else
>> +			dst_prev->output = xfrm6_output;
>> +#endif
>>  		if (rt->peer)
>>  			atomic_inc(&rt->peer->refcnt);
> 
> If it's non-NULL and you get a segfault for atomic_inc() that
> means there is garbage here, and it seems that if you're
> setting it to NULL explicitly then it's just a workaround
> for whatever problem is causing it to be non-NULL to begin
> with.
> 
> What is putting a non-valid pointer value there?  Is this an IPV6 or
> IPSEC dst route by chance?  If so, that makes this change really
> wrong, and we are corrupting the route by running rt_bind_peer() on
> it.  rt_bind_peer() is only valid on ipv4 route entries.

Thank you for your good catch.
I think atomic_inc must be done in case of props.family == AF_INET.
And we probably should manage reference count of the device in case of
AF_INET6.

Anyway I'll check and fix it.

Thank you.

--
Kazunori Miyazawa
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html