From: Kees Cook <kees@kernel.org>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Kumar Kartikeya Dwivedi <memxor@gmail.com>,
bpf <bpf@vger.kernel.org>, Emil Tsalapatis <emil@etsalapatis.com>,
Alexei Starovoitov <ast@kernel.org>,
Andrii Nakryiko <andrii@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Martin KaFai Lau <martin.lau@kernel.org>,
Eduard Zingerman <eddyz87@gmail.com>,
Barret Rhoden <brho@google.com>,
Matt Bobrowski <mattbobrowski@google.com>,
kkd@meta.com, Kernel Team <kernel-team@meta.com>,
linux-hardening@vger.kernel.org
Subject: Re: [PATCH bpf-next v5 08/12] bpf: Report rqspinlock deadlocks/timeout to BPF stderr
Date: Wed, 30 Jul 2025 16:09:33 -0700 [thread overview]
Message-ID: <202507301608.C939FE7D9@keescook> (raw)
In-Reply-To: <CAADnVQ+n-o2qeoLqvfJgY4wf9Ms-xs_SyEZhtfgkidqjX=u3qg@mail.gmail.com>
On Wed, Jul 30, 2025 at 04:07:33PM -0700, Alexei Starovoitov wrote:
> On Wed, Jul 30, 2025 at 4:02 PM Kees Cook <kees@kernel.org> wrote:
> >
> > On Thu, Jul 03, 2025 at 01:48:14PM -0700, Kumar Kartikeya Dwivedi wrote:
> > > +static void bpf_prog_report_rqspinlock_violation(const char *str, void *lock, bool irqsave)
> > > +{
> > > + struct rqspinlock_held *rqh = this_cpu_ptr(&rqspinlock_held_locks);
> > > + struct bpf_stream_stage ss;
> > > + struct bpf_prog *prog;
> > > +
> > > + prog = bpf_prog_find_from_stack();
> > > + if (!prog)
> > > + return;
> > > + bpf_stream_stage(ss, prog, BPF_STDERR, ({
> > > + bpf_stream_printk(ss, "ERROR: %s for bpf_res_spin_lock%s\n", str, irqsave ? "_irqsave" : "");
> > > + bpf_stream_printk(ss, "Attempted lock = 0x%px\n", lock);
> > > + bpf_stream_printk(ss, "Total held locks = %d\n", rqh->cnt);
> > > + for (int i = 0; i < min(RES_NR_HELD, rqh->cnt); i++)
> > > + bpf_stream_printk(ss, "Held lock[%2d] = 0x%px\n", i, rqh->locks[i]);
> > > + bpf_stream_dump_stack(ss);
> >
> > Please don't include %px in stuff going back to userspace in standard
> > error reporting. That's a kernel address leak:
> > https://docs.kernel.org/process/deprecated.html#p-format-specifier
> >
> > I don't see any justification here, please remove the lock address or
> > use regular %p to get a hashed value.
>
> There is no leak here.
> The prog was loaded by root and error is read by root.
uid has nothing to do with it. Leaking addresses needs the right
capability set. Is that always true here?
--
Kees Cook
next prev parent reply other threads:[~2025-07-30 23:09 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-07-03 20:48 [PATCH bpf-next v5 00/12] BPF Standard Streams Kumar Kartikeya Dwivedi
2025-07-03 20:48 ` [PATCH bpf-next v5 01/12] bpf: Refactor bprintf buffer support Kumar Kartikeya Dwivedi
2025-07-03 20:48 ` [PATCH bpf-next v5 02/12] bpf: Introduce BPF standard streams Kumar Kartikeya Dwivedi
2025-07-03 20:48 ` [PATCH bpf-next v5 03/12] bpf: Add function to extract program source info Kumar Kartikeya Dwivedi
2025-07-03 20:48 ` [PATCH bpf-next v5 04/12] bpf: Ensure RCU lock is held around bpf_prog_ksym_find Kumar Kartikeya Dwivedi
2025-07-03 20:48 ` [PATCH bpf-next v5 05/12] bpf: Add function to find program from stack trace Kumar Kartikeya Dwivedi
2025-07-03 20:48 ` [PATCH bpf-next v5 06/12] bpf: Add dump_stack() analogue to print to BPF stderr Kumar Kartikeya Dwivedi
2025-07-03 20:48 ` [PATCH bpf-next v5 07/12] bpf: Report may_goto timeout " Kumar Kartikeya Dwivedi
2025-07-03 20:48 ` [PATCH bpf-next v5 08/12] bpf: Report rqspinlock deadlocks/timeout " Kumar Kartikeya Dwivedi
2025-07-30 23:02 ` Kees Cook
2025-07-30 23:07 ` Alexei Starovoitov
2025-07-30 23:09 ` Kees Cook [this message]
2025-07-30 23:13 ` Alexei Starovoitov
2025-07-30 23:19 ` Kees Cook
2025-07-03 20:48 ` [PATCH bpf-next v5 09/12] libbpf: Add bpf_stream_printk() macro Kumar Kartikeya Dwivedi
2025-07-03 20:48 ` [PATCH bpf-next v5 10/12] libbpf: Introduce bpf_prog_stream_read() API Kumar Kartikeya Dwivedi
2025-07-03 20:48 ` [PATCH bpf-next v5 11/12] bpftool: Add support for dumping streams Kumar Kartikeya Dwivedi
2025-07-03 20:48 ` [PATCH bpf-next v5 12/12] selftests/bpf: Add tests for prog streams Kumar Kartikeya Dwivedi
2025-07-04 2:40 ` [PATCH bpf-next v5 00/12] BPF Standard Streams patchwork-bot+netdevbpf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=202507301608.C939FE7D9@keescook \
--to=kees@kernel.org \
--cc=alexei.starovoitov@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=brho@google.com \
--cc=daniel@iogearbox.net \
--cc=eddyz87@gmail.com \
--cc=emil@etsalapatis.com \
--cc=kernel-team@meta.com \
--cc=kkd@meta.com \
--cc=linux-hardening@vger.kernel.org \
--cc=martin.lau@kernel.org \
--cc=mattbobrowski@google.com \
--cc=memxor@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox