Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220810171702.74932-1-flaniel@linux.microsoft.com> <20220810171702.74932-2-flaniel@linux.microsoft.com>
In-Reply-To: <20220810171702.74932-2-flaniel@linux.microsoft.com>
From:   Andrii Nakryiko <andrii.nakryiko@gmail.com>
Date:   Mon, 15 Aug 2022 14:52:22 -0700
Message-ID: <CAEf4BzYex03T7aYjLnbkfHb8vUsCHhj_DiMU6KbK29F+DyhXyA@mail.gmail.com>
Subject: Re: [RFC PATCH v1 1/3] bpf: Make ring buffer overwritable.
To:     Francis Laniel <flaniel@linux.microsoft.com>
Cc:     bpf@vger.kernel.org, linux-kernel@vger.kernel.org,
        Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>,
        Martin KaFai Lau <martin.lau@linux.dev>,
        Song Liu <song@kernel.org>, Yonghong Song <yhs@fb.com>,
        John Fastabend <john.fastabend@gmail.com>,
        KP Singh <kpsingh@kernel.org>,
        Stanislav Fomichev <sdf@google.com>,
        Hao Luo <haoluo@google.com>, Jiri Olsa <jolsa@kernel.org>,
        Joanne Koong <joannelkoong@gmail.com>,
        Dave Marchevsky <davemarchevsky@fb.com>,
        Lorenzo Bianconi <lorenzo@kernel.org>,
        Geliang Tang <geliang.tang@suse.com>,
        Hengqi Chen <hengqi.chen@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Aug 10, 2022 at 10:18 AM Francis Laniel
<flaniel@linux.microsoft.com> wrote:
>
> By default, BPF ring buffer are size bounded, when producers already filled the
> buffer, they need to wait for the consumer to get those data before adding new
> ones.
> In terms of API, bpf_ringbuf_reserve() returns NULL if the buffer is full.
>
> This patch permits making BPF ring buffer overwritable.
> When producers already wrote as many data as the buffer size, they will begin to
> over write existing data, so the oldest will be replaced.
> As a result, bpf_ringbuf_reserve() never returns NULL.
>

Part of BPF ringbuf record (first 8 bytes) stores information like
record size and offset in pages to the beginning of ringbuf map
metadata. This is used by consumer to know how much data belongs to
data record, but also for making sure that
bpf_ringbuf_reserve()/bpf_ringbuf_submit() work correctly and don't
corrupt kernel memory.

If we simply allow overwriting this information (and no, spinlock
doesn't protect from that, you can have multiple producers writing to
different parts of ringbuf data area in parallel after "reserving"
their respective records), it completely breaks any sort of
correctness, both for user-space consumer and kernel-side producers.

> Signed-off-by: Francis Laniel <flaniel@linux.microsoft.com>
> ---
>  include/uapi/linux/bpf.h |  3 +++
>  kernel/bpf/ringbuf.c     | 51 +++++++++++++++++++++++++++++++---------
>  2 files changed, 43 insertions(+), 11 deletions(-)
>

[...]