[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <20100531.234432.191383736.davem@davemloft.net>
Date: Mon, 31 May 2010 23:44:32 -0700 (PDT)
From: David Miller <davem@...emloft.net>
To: eric.dumazet@...il.com
Cc: anton@...ba.org, netdev@...r.kernel.org
Subject: Re: [PATCH] net: sock_queue_err_skb() dont mess with
sk_forward_alloc
From: Eric Dumazet <eric.dumazet@...il.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@...il.com>
Looks good, applied, thanks Eric.
--
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