linux-kernel - Re: [PATCH net 3/3] ip_gre, ip6_gre: Fix race condition on o_seqno in collect

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: <CALDO+SYfemnqVkQY6kbQ23hgtdcUSimExJQob90rZLd8xvzsXw@mail.gmail.com>
Date:   Fri, 22 Apr 2022 09:35:08 -0700
From:   William Tu <u9012063@...il.com>
To:     Peilin Ye <yepeilin.cs@...il.com>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
        David Ahern <dsahern@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Peilin Ye <peilin.ye@...edance.com>,
        "xeb@...l.ru" <xeb@...l.ru>,
        Daniel Borkmann <daniel@...earbox.net>,
        Cong Wang <cong.wang@...edance.com>,
        Eric Dumazet <eric.dumazet@...il.com>,
        Alexei Starovoitov <ast@...nel.org>,
        Andrii Nakryiko <andrii@...nel.org>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...nel.org>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>,
        bpf <bpf@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net 3/3] ip_gre, ip6_gre: Fix race condition on o_seqno in
 collect_md mode

On Thu, Apr 21, 2022 at 3:09 PM Peilin Ye <yepeilin.cs@...il.com> wrote:
>
> From: Peilin Ye <peilin.ye@...edance.com>
>
> As pointed out by Jakub Kicinski, currently using TUNNEL_SEQ in
> collect_md mode is racy for [IP6]GRE[TAP] devices.  Consider the
> following sequence of events:
>
> 1. An [IP6]GRE[TAP] device is created in collect_md mode using "ip link
>    add ... external".  "ip" ignores "[o]seq" if "external" is specified,
>    so TUNNEL_SEQ is off, and the device is marked as NETIF_F_LLTX (i.e.
>    it uses lockless TX);
> 2. Someone sets TUNNEL_SEQ on outgoing skb's, using e.g.
>    bpf_skb_set_tunnel_key() in an eBPF program attached to this device;
> 3. gre_fb_xmit() or __gre6_xmit() processes these skb's:
>
>         gre_build_header(skb, tun_hlen,
>                          flags, protocol,
>                          tunnel_id_to_key32(tun_info->key.tun_id),
>                          (flags & TUNNEL_SEQ) ? htonl(tunnel->o_seqno++)
>                                               : 0);   ^^^^^^^^^^^^^^^^^
>
> Since we are not using the TX lock (&txq->_xmit_lock), multiple CPUs may
> try to do this tunnel->o_seqno++ in parallel, which is racy.  Fix it by
> making o_seqno atomic_t.
>
> As mentioned by Eric Dumazet in commit b790e01aee74 ("ip_gre: lockless
> xmit"), making o_seqno atomic_t increases "chance for packets being out
> of order at receiver" when NETIF_F_LLTX is on.
>
> Maybe a better fix would be:
>
> 1. Do not ignore "oseq" in external mode.  Users MUST specify "oseq" if
>    they want the kernel to allow sequencing of outgoing packets;
> 2. Reject all outgoing TUNNEL_SEQ packets if the device was not created
>    with "oseq".
>
> Unfortunately, that would break userspace.
>
> We could now make [IP6]GRE[TAP] devices always NETIF_F_LLTX, but let us
> do it in separate patches to keep this fix minimal.
>
> Suggested-by: Jakub Kicinski <kuba@...nel.org>
> Fixes: 77a5196a804e ("gre: add sequence number for collect md mode.")
> Signed-off-by: Peilin Ye <peilin.ye@...edance.com>
> ---

LGTM
Acked-by: William Tu <u9012063@...il.com>