netdev - Re: [PATCH] net: sock_queue_err_skb() dont mess with sk_forward

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] [day] [month] [year] [list]

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