netdev - Re: race in skb_splice

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]

Date:	Tue, 27 May 2008 19:47:10 +0400
From:	Evgeniy Polyakov <johnpol@....mipt.ru>
To:	Octavian Purdila <opurdila@...acom.com>
Cc:	Ben Hutchings <bhutchings@...arflare.com>, netdev@...r.kernel.org,
	davem@...emloft.net
Subject: Re: race in skb_splice_bits?

On Tue, May 27, 2008 at 06:33:55PM +0300, Octavian Purdila (opurdila@...acom.com) wrote:
> Yes, I think I got the idea you are trying to use here. But, somehow I feel 
> uneasy with this approach :) Isn't it cleaner to keep the lock and try to 
> avoid the deadlock on the sendfile() side? Or is that unfeasible?

Well, providing some flags to ->sendpage() callbacks to say that they
should not grab locks is a bit ugly to me, I think we can fix splice
code not to do wrong things.

> I don't think we can drop the socket lock, it will introduce at least on type 
> of races: since the skb we are processing is still on the socket queue, any 
> entity accessing the socket queue will possible collide with us. 

Each access is being done under socket lock, so it should be safe. But
socket release path drops skb from the list before dropping its
reference counter, so we are not allowed to unlink it again, but we can
check if skb is linked or not and make a decision based on that.

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