netdev - RE: [EXTERNAL] Re: [PATCH bpf-next v2] Update perf ring buffer to prevent corruption

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]

Date:   Mon, 9 Nov 2020 14:22:28 +0000
From:   Kevin Sheldrake <Kevin.Sheldrake@...rosoft.com>
To:     Peter Zijlstra <peterz@...radead.org>,
        Alexei Starovoitov <alexei.starovoitov@...il.com>
CC:     Ingo Molnar <mingo@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Network Development <netdev@...r.kernel.org>,
        "bpf@...r.kernel.org" <bpf@...r.kernel.org>,
        Andrii Nakryiko <andrii.nakryiko@...il.com>,
        KP Singh <kpsingh@...gle.com>
Subject: RE: [EXTERNAL] Re: [PATCH bpf-next v2] Update perf ring buffer to
 prevent corruption



> -----Original Message-----
> From: Peter Zijlstra <peterz@...radead.org>
> Sent: 09 November 2020 11:29
> To: Alexei Starovoitov <alexei.starovoitov@...il.com>
> Cc: Kevin Sheldrake <Kevin.Sheldrake@...rosoft.com>; Ingo Molnar
> <mingo@...nel.org>; Daniel Borkmann <daniel@...earbox.net>; Network
> Development <netdev@...r.kernel.org>; bpf@...r.kernel.org; Andrii
> Nakryiko <andrii.nakryiko@...il.com>; KP Singh <kpsingh@...gle.com>
> Subject: [EXTERNAL] Re: [PATCH bpf-next v2] Update perf ring buffer to
> prevent corruption
> 
> On Thu, Nov 05, 2020 at 08:19:47PM -0800, Alexei Starovoitov wrote:
> > On Thu, Nov 5, 2020 at 7:18 AM Kevin Sheldrake
> > <Kevin.Sheldrake@...rosoft.com> wrote:
> > >
> > > Resent due to some failure at my end.  Apologies if it arrives twice.
> > >
> > > From 63e34d4106b4dd767f9bfce951f8a35f14b52072 Mon Sep 17 00:00:00
> 2001
> > > From: Kevin Sheldrake <kevin.sheldrake@...rosoft.com>
> > > Date: Thu, 5 Nov 2020 12:18:53 +0000
> > > Subject: [PATCH] Update perf ring buffer to prevent corruption from
> > >  bpf_perf_output_event()
> > >
> > > The bpf_perf_output_event() helper takes a sample size parameter of
> u64, but
> > > the underlying perf ring buffer uses a u16 internally. This 64KB maximum
> size
> > > has to also accommodate a variable sized header. Failure to observe this
> > > restriction can result in corruption of the perf ring buffer as samples
> > > overlap.
> > >
> > > Track the sample size and return -E2BIG if too big to fit into the u16
> > > size parameter.
> > >
> > > Signed-off-by: Kevin Sheldrake <kevin.sheldrake@...rosoft.com>
> >
> > The fix makes sense to me.
> > Peter, Ingo,
> > should I take it through the bpf tree or you want to route via tip?
> 
> What are you doing to trigger this? The Changelog is devoid of much
> useful information?

Hello

I triggered the corruption by sending samples larger than 64KB-24 bytes
to a perf ring buffer from eBPF using bpf_perf_event_output().  The u16
that holds the size in the struct perf_event_header is overflowed and
the distance between adjacent samples in the perf ring buffer is set
by this overflowed value; hence if samples of 64KB are sent, adjacent
samples are placed 24 bytes apart in the ring buffer, with the later ones
overwriting parts of the earlier ones.  If samples aren't read as quickly
as they are received, then they are corrupted by the time they are read.

Attempts to fix this in the eBPF verifier failed as the actual sample is
constructed from a variable sized header in addition to the raw data
supplied from eBPF.  The sample is constructed in perf_prepare_sample(),
outside of the eBPF engine.

My proposed fix is to check that the constructed size is <U16_MAX before
committing it to the struct perf_event_header::size variable.

A reproduction of the bug can be found at:
https://github.com/microsoft/OMS-Auditd-Plugin/tree/MSTIC-Research/ebpf_perf_output_poc

Thanks

Kevin Sheldrake