netdev - Re: [bpf PATCH 0/3] bpf sockmap fixes

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:   Thu, 25 Jan 2018 17:06:15 -0800
From:   John Fastabend <john.fastabend@...il.com>
To:     ast@...nel.org, daniel@...earbox.net, davejwatson@...com
Cc:     netdev@...r.kernel.org, bhole_prashant_q7@....ntt.co.jp
Subject: Re: [bpf PATCH 0/3] bpf sockmap fixes

On 01/25/2018 04:26 PM, John Fastabend wrote:
> A set of fixes for sockmap to resolve map/prog not being cleaned
> up when maps are not cleaned up from the program side.
> 

Well that first sentence is a bit confusing now that I read it again.
Here is a better version,

"
A set of fixes for sockmap to resolve programs referencing sockmaps
and closing without deleting all entries in the map and/or not detaching
BPF programs attached to the map. Both leaving entries in the map and
not detaching programs may result in the map failing to be removed by
BPF infrastructure due to reference counts never reaching zero.
"


> For this we pull in the ULP infrastructure to hook into the close()
> hook of the sock layer. This seemed natural because we have additional
> sockmap features (to add support for TX hooks) that will also use the
> ULP infrastructure. This allows us to cleanup entries in the map when
> socks are closed() and avoid trying to get the sk_state_change() hook
> to fire in all cases.
> 
> The second issue resolved here occurs when users don't detach
> programs. The gist is a refcnt issue resolved by implementing the
> release callback. See patch for details.
> 
> For testing I ran both sample/sockmap and selftests bpf/test_maps.c.
> I did not however retest TLS with the small change to ULP layer.
> Mostly because I don't have a TLS setup. I plan/hope to get around
> to writing either a sample or preferably a selftest for this case
> as well (assuming I didn't miss one).
> 
> @Dave Watson can you take a quick look to verify the changes are
> good on TLS ULP side.
> 
> ---
> 
> John Fastabend (3):
>       net: add a UID to use for ULP socket assignment
>       bpf: sockmap, add sock close() hook to remove socks
>       bpf: sockmap, fix leaking maps with attached but not detached progs
> 
> 
>  include/net/tcp.h    |    8 ++
>  kernel/bpf/sockmap.c |  164 ++++++++++++++++++++++++++++----------------------
>  net/ipv4/tcp_ulp.c   |   53 +++++++++++++++-
>  net/tls/tls_main.c   |    2 +
>  4 files changed, 150 insertions(+), 77 deletions(-)
> 
> --
> Signature
>