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  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, 13 Jul 2009 19:46:51 +0200
From:	Oliver Hartkopp <oliver@...tkopp.net>
To:	Lothar Waßmann <LW@...O-electronics.de>
CC:	Herbert Xu <herbert@...dor.apana.org.au>, davem@...emloft.net,
	netdev@...r.kernel.org, urs.thuermann@...kswagen.de,
	Urs Thuermann <urs@...ogud.escape.de>
Subject: Re: use after free bug in socket code

Lothar Waßmann wrote:
> Herbert Xu writes:
>> Lothar Waßmann <LW@...o-electronics.de> wrote:
>>> So, could you point me to the place where the reference count of the
>>> socket object is being incremented when a struct sock is associated
>>> with it?
>> It's implicit.  Anyway, you should remodel your release function
>> on a working protocol.
>>
> OK. I checked the release functions of the can raw and bcm protocols
> and found that they obviously are the culprits since they lack the
> call to sock_orphan that other network protocol drivers have:
> 
> diff -ur linux-2.6.30/net/can/bcm.c linux-2.6.30-karo/net/can/bcm.c
> --- linux-2.6.30/net/can/bcm.c	2009-06-10 05:05:27.000000000 +0200
> +++ linux-2.6.30-karo/net/can/bcm.c	2009-07-12 20:12:38.000000000 +0200
> @@ -1469,6 +1469,9 @@
>  		bo->ifindex = 0;
>  	}
>  
> +	sock_orphan(sk);
> +	sock->sk = NULL;
> +
>  	release_sock(sk);
>  	sock_put(sk);
>  
> diff -ur linux-2.6.30/net/can/raw.c linux-2.6.30-karo/net/can/raw.c
> --- linux-2.6.30/net/can/raw.c	2009-06-10 05:05:27.000000000 +0200
> +++ linux-2.6.30-karo/net/can/raw.c	2009-07-12 20:12:29.000000000 +0200
> @@ -306,6 +306,9 @@
>  	ro->bound   = 0;
>  	ro->count   = 0;
>  
> +	sock_orphan(sk);
> +	sock->sk = NULL;
> +
>  	release_sock(sk);
>  	sock_put(sk);
>  
> 
> 
> Could someone of the CAN-Folks comment on this?

Hello Lothar,

unfortunately i did not get any answer from Urs so far, who originally created
these lines of code.

But from what i was able to get from browsing similar code in the Kernel that
at least sock_orphan() is called in the appropriate socket release functions,
which is indeed not done by the mentioned PF_CAN protocols right now.

I assume you already tested this patch (at least with CAN_RAW) successfully,
right?

If so, i would have no objections to add my Acked-by to these changes.

Would you like to prepare a proper patch and post it on netdev?

Thanks for digging that issue!

Best regards,
Oliver

ps. This code section was stable for more than three years now. Can you tell
me, how you kicked your system to run into this problem?


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

Powered by blists - more mailing lists