linux-kernel - Re: [PATCH] sched/tracing: append prev

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: <CAADnVQJTZ=QK+G6KU9GW2M7busBQEFbHUYRQsxi_a=ZpATBsyw@mail.gmail.com>
Date:   Wed, 27 Apr 2022 13:32:39 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc:     Qais Yousef <qais.yousef@....com>,
        Peter Zijlstra <peterz@...radead.org>,
        Delyan Kratunov <delyank@...com>,
        Namhyung Kim <namhyung@...nel.org>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        "bigeasy@...utronix.de" <bigeasy@...utronix.de>,
        "dietmar.eggemann@....com" <dietmar.eggemann@....com>,
        "keescook@...omium.org" <keescook@...omium.org>,
        "x86@...nel.org" <x86@...nel.org>,
        "andrii@...nel.org" <andrii@...nel.org>,
        "u.kleine-koenig@...gutronix.de" <u.kleine-koenig@...gutronix.de>,
        "vincent.guittot@...aro.org" <vincent.guittot@...aro.org>,
        "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
        "mingo@...nel.org" <mingo@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "rdunlap@...radead.org" <rdunlap@...radead.org>,
        "rostedt@...dmis.org" <rostedt@...dmis.org>,
        "Kenta.Tada@...y.com" <Kenta.Tada@...y.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "bristot@...hat.com" <bristot@...hat.com>,
        "ebiederm@...ssion.com" <ebiederm@...ssion.com>,
        "ast@...nel.org" <ast@...nel.org>,
        "legion@...nel.org" <legion@...nel.org>,
        "adharmap@...cinc.com" <adharmap@...cinc.com>,
        "valentin.schneider@....com" <valentin.schneider@....com>,
        "ed.tsai@...iatek.com" <ed.tsai@...iatek.com>,
        "juri.lelli@...hat.com" <juri.lelli@...hat.com>
Subject: Re: [PATCH] sched/tracing: append prev_state to tp args instead

On Wed, Apr 27, 2022 at 11:17 AM Andrii Nakryiko
<andrii.nakryiko@...il.com> wrote:
>
> > >
> > > See my reply to Peter. libbpf can't know user's intent to fail this
> > > automatically, in general. In some cases when it can it does
> > > accommodate this automatically. In other cases it provides instruments
> > > for user to handle this (bpf_core_field_size(),
> > > BPF_CORE_READ_BITFIELD(), etc).
> >
> > My naiive thinking is that the function signature has changed (there's 1 extra
> > arg not just a subtle swap of args of the same type) - so I thought that can be
> > detected. But maybe it is harder said than done.
>
> It is. We don't have number of arguments either:
>
> struct bpf_raw_tracepoint_args {
>         __u64 args[0];
> };
>
> What BPF program is getting is just an array of u64s.

Well, that's a true and false statement at the same time
that might confuse folks reading this thread.
To clarify:
raw_tp and tp_btf programs receive an array of u64-s.
raw_tp checks the number of arguments only.
The prog that accesses non-existing arg will be rejected.
tp_btf prog in addition to the number of args knows
the exact type of the arguments.
So in this case accessing arg0 in bpf prog
as 'struct task_struct *' while it's 'unsigned int prev_state'
in the kernel will cause the verifier to reject it.
If prog does bpf_probe_read_kernel() then all bets are off, of course.
There is no type checking mechanism for bpf_probe_read_kernel.
Only BTF powered pointer dereferences are type checked.
The 'tp_btf' prog type is the recommended way to access tracepoints.
The 'raw_tp' was implemented before BTF was introduced.