netdev - Re: netns_id in bpf_sk_lookup

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: <077323f3-3026-06b5-fb4b-54383e5387f6@gmail.com>
Date:   Mon, 19 Nov 2018 16:46:23 -0700
From:   David Ahern <dsahern@...il.com>
To:     Joe Stringer <joe@...d.net.nz>
Cc:     nicolas.dichtel@...nd.com, netdev <netdev@...r.kernel.org>,
        daniel@...earbox.net
Subject: Re: netns_id in bpf_sk_lookup_{tcp,udp}

On 11/19/18 2:59 PM, Joe Stringer wrote:
> @@ -2221,12 +2222,13 @@ union bpf_attr {
>  *             **sizeof**\ (*tuple*\ **->ipv6**)
>  *                     Look for an IPv6 socket.
>  *
> - *             If the *netns* is zero, then the socket lookup table in the
> - *             netns associated with the *ctx* will be used. For the TC hooks,
> - *             this in the netns of the device in the skb. For socket hooks,
> - *             this in the netns of the socket. If *netns* is non-zero, then
> - *             it specifies the ID of the netns relative to the netns
> - *             associated with the *ctx*.
> + *             If the *netns* is **BPF_F_SK_CURRENT_NS** or greater, then the
> + *             socket lookup table in the netns associated with the *ctx* will
> + *             will be used. For the TC hooks, this is the netns of the device
> + *             in the skb. For socket hooks, this is the netns of the socket.
> + *             If *netns* is less than **BPF_F_SK_CURRENT_NS**, then it
> + *             specifies the ID of the netns relative to the netns associated
> + *             with the *ctx*.
>  *
>  *             All values for *flags* are reserved for future usage, and must
>  *             be left at zero.
> @@ -2409,6 +2411,9 @@ enum bpf_func_id {
> /* BPF_FUNC_perf_event_output for sk_buff input context. */
> #define BPF_F_CTXLEN_MASK              (0xfffffULL << 32)
> 
> +/* BPF_FUNC_sk_lookup_tcp and BPF_FUNC_sk_lookup_udp flags. */
> +#define BPF_F_SK_CURRENT_NS            0x80000000 /* For netns argument */
> +
> /* Mode for BPF_FUNC_skb_adjust_room helper. */
> enum bpf_adj_room_mode {
>        BPF_ADJ_ROOM_NET,
> 
> Plus adjusting all of the internal types and the helper headers to use
> u32. With the highest bit used to specify that the netns should be the
> current netns, all other netns IDs should be available.
> 

That seems reasonable if the nsid limit is s32.

That revelation shows another hole:
$ ip netns add foo
$ ip netns set foo 0xffffffff
$ ip netns list
foo (id: 0)

Seems like alloc_netid() should error out if reqid < -1 (-1 being the
NETNSA_NSID_NOT_ASSIGNED flag) as opposed to blindly ignoring it.