netdev - Re: [PATCH bpf-next 1/2] bpf: Add sock ops get netns helpers

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: <CAGdtWsQEvnOuZatftxmqp5iT_ac3jCjEV6q2gWaJRbP_TpDDcg@mail.gmail.com>
Date:   Thu, 6 Feb 2020 10:48:46 -0800
From:   Petar Penkov <ppenkov.kernel@...il.com>
To:     Lingpeng Chen <forrest0579@...il.com>
Cc:     bpf <bpf@...r.kernel.org>, Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        John Fastabend <john.fastabend@...il.com>,
        "David S . Miller" <davem@...emloft.net>,
        Networking <netdev@...r.kernel.org>
Subject: Re: [PATCH bpf-next 1/2] bpf: Add sock ops get netns helpers

On Thu, Feb 6, 2020 at 12:36 AM Lingpeng Chen <forrest0579@...il.com> wrote:
>
> Currently 5-tuple(sip+dip+sport+dport+proto) can't identify a
> uniq connection because there may be multi net namespace.
> For example, there may be a chance that netns a and netns b all
> listen on 127.0.0.1:8080 and the client with same port 40782
> connect to them. Without netns number, sock ops program
> can't distinguish them.
> Using bpf_sock_ops_get_netns helpers to get current connection
> netns number to distinguish connections.
>
> Signed-off-by: Lingpeng Chen <forrest0579@...il.com>
> ---
>  include/uapi/linux/bpf.h |  8 +++++++-
>  net/core/filter.c        | 18 ++++++++++++++++++
>  2 files changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index f1d74a2bd234..b15a55051232 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -2892,6 +2892,11 @@ union bpf_attr {
>   *             Obtain the 64bit jiffies
>   *     Return
>   *             The 64 bit jiffies
> + * u32 bpf_sock_ops_get_netns(struct bpf_sock_ops *bpf_socket)
Should this return u64 instead? Also, would it make sense here to add
a 'u64 flags' field to allow some extensibility of this helper
function, consistent with some other helpers. Initially, we can reject
any flags with:
if (unlikely(flags))
        return -EINVAL;

Last, I was hoping we could add a regression test for this helper.

Thanks,
Petar

> + *  Description
> + *      Obtain netns id of sock
> + * Return
> + *      The current netns inum
>   */
>  #define __BPF_FUNC_MAPPER(FN)          \
>         FN(unspec),                     \
> @@ -3012,7 +3017,8 @@ union bpf_attr {
>         FN(probe_read_kernel_str),      \
>         FN(tcp_send_ack),               \
>         FN(send_signal_thread),         \
> -       FN(jiffies64),
> +       FN(jiffies64),          \
> +       FN(sock_ops_get_netns),
>
>  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
>   * function eBPF program intends to call
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 792e3744b915..b7f33f20e8fb 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -4421,6 +4421,22 @@ static const struct bpf_func_proto bpf_sock_ops_cb_flags_set_proto = {
>         .arg2_type      = ARG_ANYTHING,
>  };
>
> +BPF_CALL_1(bpf_sock_ops_get_netns, struct bpf_sock_ops_kern *, bpf_sock)
> +{
> +       struct sock *sk = bpf_sock->sk;
> +
> +       if (!IS_ENABLED(CONFIG_NET_NS))
> +               return 0;
> +       return sk->sk_net.net->ns.inum;
> +}
> +
> +static const struct bpf_func_proto bpf_sock_ops_get_netns_proto = {
> +       .func           = bpf_sock_ops_get_netns,
> +       .gpl_only       = false,
> +       .ret_type       = RET_INTEGER,
> +       .arg1_type      = ARG_PTR_TO_CTX,
> +};
> +
>  const struct ipv6_bpf_stub *ipv6_bpf_stub __read_mostly;
>  EXPORT_SYMBOL_GPL(ipv6_bpf_stub);
>
> @@ -6218,6 +6234,8 @@ sock_ops_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>         case BPF_FUNC_tcp_sock:
>                 return &bpf_tcp_sock_proto;
>  #endif /* CONFIG_INET */
> +       case BPF_FUNC_sock_ops_get_netns:
> +               return &bpf_sock_ops_get_netns_proto;
>         default:
>                 return bpf_base_func_proto(func_id);
>         }
> --
> 2.17.1
>