netdev - Re: [PATCH v3 bpf-next 4/9] tcp: Add unknown_opt arg to tcp_parse

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: <CANn89i+f4se896OPGx6dPKZuObeJR2gaTExqoAHmDK=r7cTmaw@mail.gmail.com>
Date:   Fri, 31 Jul 2020 09:12:10 -0700
From:   Eric Dumazet <edumazet@...gle.com>
To:     Martin KaFai Lau <kafai@...com>
Cc:     bpf <bpf@...r.kernel.org>, Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        kernel-team <kernel-team@...com>,
        Lawrence Brakmo <brakmo@...com>,
        Neal Cardwell <ncardwell@...gle.com>,
        netdev <netdev@...r.kernel.org>,
        Yuchung Cheng <ycheng@...gle.com>
Subject: Re: [PATCH v3 bpf-next 4/9] tcp: Add unknown_opt arg to tcp_parse_options

On Thu, Jul 30, 2020 at 1:58 PM Martin KaFai Lau <kafai@...com> wrote:
>
> In the latter patch, the bpf prog only wants to be called to handle
> a header option if that particular header option cannot be handled by
> the kernel.  This unknown option could be written by the peer's bpf-prog.
> It could also be a new standard option that the running kernel does not
> support it while a bpf-prog can handle it.
>
> In a latter patch, the bpf prog will be called from tcp_validate_incoming()
> if there is unknown option and a flag is set in tp->bpf_sock_ops_cb_flags.
>
> Instead of using skb->cb[] in an earlier attempt, this patch
> adds an optional arg "bool *unknown_opt" to tcp_parse_options().
> The bool will be set to true if it has encountered an option
> that the kernel does not recognize.
>
> Signed-off-by: Martin KaFai Lau <kafai@...com>
> ---
>  drivers/infiniband/hw/cxgb4/cm.c |  2 +-
>  include/net/tcp.h                |  3 ++-
>  net/ipv4/syncookies.c            |  2 +-
>  net/ipv4/tcp_input.c             | 40 +++++++++++++++++++++-----------
>  net/ipv4/tcp_minisocks.c         |  4 ++--
>  net/ipv6/syncookies.c            |  2 +-
>  6 files changed, 34 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c
> index 30e08bcc9afb..dedca6576bb9 100644
> --- a/drivers/infiniband/hw/cxgb4/cm.c
> +++ b/drivers/infiniband/hw/cxgb4/cm.c
> @@ -3949,7 +3949,7 @@ static void build_cpl_pass_accept_req(struct sk_buff *skb, int stid , u8 tos)
>          */
>         memset(&tmp_opt, 0, sizeof(tmp_opt));
>         tcp_clear_options(&tmp_opt);
> -       tcp_parse_options(&init_net, skb, &tmp_opt, 0, NULL);
> +       tcp_parse_options(&init_net, skb, &tmp_opt, 0, NULL, NULL);
>
>         req = __skb_push(skb, sizeof(*req));
>         memset(req, 0, sizeof(*req));
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 895e7aabf136..d49d8f1c961a 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -413,7 +413,8 @@ int tcp_mmap(struct file *file, struct socket *sock,
>  #endif
>  void tcp_parse_options(const struct net *net, const struct sk_buff *skb,
>                        struct tcp_options_received *opt_rx,
> -                      int estab, struct tcp_fastopen_cookie *foc);
> +                      int estab, struct tcp_fastopen_cookie *foc,
> +                      bool *unknown_opt);
>  const u8 *tcp_parse_md5sig_option(const struct tcphdr *th);
>

Instead of changing signatures of many functions (and make future
stable backports challenging)
how about adding a field into 'struct tcp_options_received' ?

Sorry for not suggesting this earlier :/