linux-kernel - Re: [PATCH] bpf: defer printk() inside __bpf_prog

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: Wed, 26 Jun 2024 18:33:11 -0400
From: Steven Rostedt <rostedt@...dmis.org>
To: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
Cc: Alexei Starovoitov <alexei.starovoitov@...il.com>, John Ogness
 <john.ogness@...utronix.de>, Alexei Starovoitov <ast@...nel.org>, Daniel
 Borkmann <daniel@...earbox.net>, Andrii Nakryiko <andrii@...nel.org>,
 Martin KaFai Lau <martin.lau@...ux.dev>, Eduard Zingerman
 <eddyz87@...il.com>, Song Liu <song@...nel.org>, Yonghong Song
 <yonghong.song@...ux.dev>, John Fastabend <john.fastabend@...il.com>, KP
 Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...ichev.me>, Hao Luo
 <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>, Petr Mladek
 <pmladek@...e.com>, Sergey Senozhatsky <senozhatsky@...omium.org>, bpf
 <bpf@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] bpf: defer printk() inside __bpf_prog_run()

On Thu, 27 Jun 2024 07:15:25 +0900
Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp> wrote:

> On 2024/06/27 1:27, Steven Rostedt wrote:
> > On Wed, 26 Jun 2024 09:02:22 +0900
> > Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp> wrote:
> >   
> >> On 2024/06/26 8:56, Alexei Starovoitov wrote:  
> >>> You are missing the point. The bug has nothing to do with bpf.    
> >>
> >> The bug is caused by calling tracing hooks with rq lock held.
> >> If tracing hooks do not exist, this bug does not exist.  
> > 
> > Could you expand on this. What tracing hooks are called with rq lock
> > held? You mean the scheduling events?  
> 
> Yes, trace_sched_switch().
> __schedule() calls trace_sched_switch() hook with rq lock held.
> 
>  #2: ffff8880b943e798 (&rq->__lock){-.-.}-{2:2}, at: raw_spin_rq_lock_nested+0x2a/0x140 kernel/sched/core.c:559
> 
>  __bpf_prog_run include/linux/filter.h:691 [inline]
>  bpf_prog_run include/linux/filter.h:698 [inline]
>  __bpf_trace_run kernel/trace/bpf_trace.c:2403 [inline]
>  bpf_trace_run4+0x334/0x590 kernel/trace/bpf_trace.c:2446
>  __traceiter_sched_switch+0x98/0xd0 include/trace/events/sched.h:222
>  trace_sched_switch include/trace/events/sched.h:222 [inline]
>  __schedule+0x2587/0x4a20 kernel/sched/core.c:6742
>  preempt_schedule_notrace+0x100/0x140 kernel/sched/core.c:7017

So you are saying that because a BPF hook can attach to a tracepoint
that is called with rq locks held, it should always disable preemption
and call printk_deferred_enter(), because it *might* hit an error path
that will call printk?? In other words, how the BPF hook is used
determines if the rq lock is held or not when it is called.

I can use that same argument for should_fail_ex(). Because how it is
used determines if the rq lock is held or not when it is called. And it
is the function that actually calls printk().

Sorry, but it makes no sense to put the burden of the
printk_deferred_enter() on the BPF hook logic. It should sit solely
with the code that actually calls printk().

-- Steve