netdev - Re: use after free bug in socket code

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: <4A5B730B.8090902@hartkopp.net>
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