Re: [PATCH] net: sock_queue_err_skb() dont mess with sk_forward_alloc

[David Miller <davem@davemloft.net>]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 31 May 2010 18:02:46 +0200

> There is also a problem in ip_recv_error(), not called with socket
> locked, skb freed -> potential corruption.
> 
> If current socket is 'owned' by a user thread, then we can still corrupt
> sk_forward_alloc, even if we use bh_lock_sock()
> 
> I dont think we need to have another backlog for such case, maybe we
> could account for skb->truesize in sk_rmem_alloc (this is atomic), and
> not account for sk_mem_charge ?

That sounds fine to me.

> [PATCH] net: sock_queue_err_skb() dont mess with sk_forward_alloc
> 
> Correct sk_forward_alloc handling for error_queue would need to use a
> backlog of frames that softirq handler could not deliver because socket
> is owned by user thread. Or extend backlog processing to be able to
> process normal and error packets.
> 
> Another possibility is to not use mem charge for error queue, this is
> what I implemented in this patch.
> 
> Note: this reverts commit 29030374
> (net: fix sk_forward_alloc corruptions), since we dont need to lock
> socket anymore.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Looks good, applied, thanks Eric.