netdev - Re: [bpf-next PATCH 02/16] sockmap: convert refcnt to an atomic refcnt

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: <20180305.163428.1811826895016433265.davem@davemloft.net>
Date:   Mon, 05 Mar 2018 16:34:28 -0500 (EST)
From:   David Miller <davem@...emloft.net>
To:     john.fastabend@...il.com
Cc:     ast@...nel.org, daniel@...earbox.net, netdev@...r.kernel.org,
        davejwatson@...com
Subject: Re: [bpf-next PATCH 02/16] sockmap: convert refcnt to an atomic
 refcnt

From: John Fastabend <john.fastabend@...il.com>
Date: Mon, 05 Mar 2018 11:51:06 -0800

> The sockmap refcnt up until now has been wrapped in the
> sk_callback_lock(). So its not actually needed any locking of its
> own. The counter itself tracks the lifetime of the psock object.
> Sockets in a sockmap have a lifetime that is independent of the
> map they are part of. This is possible because a single socket may
> be in multiple maps. When this happens we can only release the
> psock data associated with the socket when the refcnt reaches
> zero. There are three possible delete sock reference decrement
> paths first through the normal sockmap process, the user deletes
> the socket from the map. Second the map is removed and all sockets
> in the map are removed, delete path is similar to case 1. The third
> case is an asyncronous socket event such as a closing the socket. The
> last case handles removing sockets that are no longer available.
> For completeness, although inc does not pose any problems in this
> patch series, the inc case only happens when a psock is added to a
> map.
> 
> Next we plan to add another socket prog type to handle policy and
> monitoring on the TX path. When we do this however we will need to
> keep a reference count open across the sendmsg/sendpage call and
> holding the sk_callback_lock() here (on every send) seems less than
> ideal, also it may sleep in cases where we hit memory pressure.
> Instead of dealing with these issues in some clever way simply make
> the reference counting a refcnt_t type and do proper atomic ops.
> 
> Signed-off-by: John Fastabend <john.fastabend@...il.com>

Acked-by: David S. Miller <davem@...emloft.net>