netdev - RE: [PATCH bpf-next v2 11/11] selftests/bpf: Tests for SOCKMAP holding listening sockets

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: <5e1a712c16d1_76782ace374ba5c02b@john-XPS-13-9370.notmuch>
Date:   Sat, 11 Jan 2020 17:06:52 -0800
From:   John Fastabend <john.fastabend@...il.com>
To:     Jakub Sitnicki <jakub@...udflare.com>, bpf@...r.kernel.org
Cc:     netdev@...r.kernel.org, kernel-team@...udflare.com,
        Eric Dumazet <edumazet@...gle.com>,
        John Fastabend <john.fastabend@...il.com>,
        Lorenz Bauer <lmb@...udflare.com>,
        Martin KaFai Lau <kafai@...com>
Subject: RE: [PATCH bpf-next v2 11/11] selftests/bpf: Tests for SOCKMAP
 holding listening sockets

Jakub Sitnicki wrote:
> Now that SOCKMAP can store listening sockets, user-space and BPF API is
> open to a new set of potential pitfalls. Exercise the map operations (with
> extra attention to code paths susceptible to races between map ops and
> socket cloning), and BPF helpers that work with SOCKMAP to gain confidence
> that all works as expected.
> 
> Signed-off-by: Jakub Sitnicki <jakub@...udflare.com>
> ---

[...]

> +static void test_sockmap_insert_listening(int family, int sotype, int mapfd)
> +{
> +	u64 value;
> +	u32 key;
> +	int s;
> +
> +	s = listen_loopback(family, sotype);
> +	if (s < 0)
> +		return;

Will the test be marked OK if listen fails here? Should we mark it skipped or
maybe even failed? Just concerned it may be passing even if the update doesn't
actually happen.

> +
> +	key = 0;
> +	value = s;
> +	xbpf_map_update_elem(mapfd, &key, &value, BPF_NOEXIST);
> +	xclose(s);
> +}

Thanks,
John