All of lore.kernel.org
 help / color / mirror / Atom feed
From: Justin Suess <utilityemal77@gmail.com>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: sashiko@lists.linux.dev, bpf <bpf@vger.kernel.org>
Subject: Re: [PATCH bpf-next v3 1/2] bpf: Implement dtor for struct file BTF ID
Date: Fri, 24 Apr 2026 22:17:32 -0400	[thread overview]
Message-ID: <aewkPGl6vkcWS5z9@zenbox> (raw)
In-Reply-To: <CAADnVQJEXDh6=rKfUjuR_zjCcXfMAXUaXcXTWgUAx1-mWe_e-w@mail.gmail.com>

On Fri, Apr 24, 2026 at 06:25:31PM -0700, Alexei Starovoitov wrote:
> On Fri, Apr 24, 2026 at 4:20 PM Justin Suess <utilityemal77@gmail.com> wrote:
> >
> >
> > So either:
> >
> > 1. allocating memory for a new irq_work on the fly, which is an
> > operation that can fail. So if we're under memory pressure the
> > element never gets freed.
> >
> > 2. preallocate an irq work for every element ahead of time.
> 
> obviously that's not what I was referring to.
> irq_work is rarely used directly like that.
> The common pattern is:
> defer_free()
> {
>         if (llist_add(head + s->offset, &df->objects))
>                 irq_work_queue(&df->work);
>
That's smart. So you can use it like a doorbell pattern and avoid
re-calling it. And avoid locking with llist.
> but before going there.
> Please enumerate "other dtors broken in NMI".
> What exactly is broken?
> What is the sequence of events?
> All map types with kptrs? or particular ones?

Only referenced kptrs.

I only tested hashmaps, not any other map types. But I don't see any
reason why this wouldn't apply to other map types.

Here are the specific dtors that know/suspect are buggy in NMI.

1. bpf_task_struct_release_dtor I confirmed. See below

2. bpf_kfree_skb_dtor can do call_rcu_hurry.

3. bpf_crypto_ctx_release_dtor does call_rcu.

4. bpf_cgroup_release_dtor frees via work queue and takes cgroup_mutex.

bpf_cpumask_release_dtor I think is safe.

I made a reproducer for one of these (task_struct dtor).

I triggered the issue by deleting the last reference to a task_struct kptr within the tp_bpf/nmi_handler prog.

`bpf_map_delete_elem()`
`  -> htab_map_delete_elem()`
`  -> free_htab_elem()`
`  -> bpf_obj_free_fields()`
`  -> bpf_task_release_dtor()`
`  -> put_task_struct_rcu_user()`
`  -> call_rcu()`

Log:
  [    1.358433] ================================
  [    1.358433] WARNING: inconsistent lock state
  [    1.358434] 7.0.0-11169-ge4ef174588b8-dirty #16 Tainted: G           OE      
  [    1.358435] --------------------------------
  [    1.358436] inconsistent {INITIAL USE} -> {IN-NMI} usage.
  [    1.358436] test_progs/134 [HC1[1]:SC0[0]:HE0:SE1] takes:
  [    1.358438] ffff8ae3bbc6f0e8 (&rdp->nocb_lock){....}-{2:2}, at: __call_rcu_common.constprop.0+0x316/0x740
  
This is done by loading a task_struct referenced kptr into a map and
deleting it later from within a tp_btf:nmi_handler BPF_PROG_TYPE_TRACING
program.

For the task_struct case;

I have a bug report:
https://lore.kernel.org/bpf/20260421201035.1729473-1-utilityemal77@gmail.com/

With reproducer:
https://gist.githubusercontent.com/RazeLighter777/5539336d79ab1854f9e9550c6dcab118/raw/082f1eeb2dd445936e64dd3a33861764690bde82/task_struct_dtor_deadlock.patch

I didn't make a reproducer for other cases.

Thanks for the tip with llist and irq_work, that blew my mind a bit.

Justin

  reply	other threads:[~2026-04-25  2:17 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-24 19:22 [PATCH bpf-next v3 0/2] Allow storing referenced struct file kptrs in BPF maps Justin Suess
2026-04-24 19:22 ` [PATCH bpf-next v3 1/2] bpf: Implement dtor for struct file BTF ID Justin Suess
2026-04-24 19:52   ` bot+bpf-ci
2026-04-24 19:59   ` sashiko-bot
2026-04-24 20:12     ` Justin Suess
2026-04-24 22:05       ` Alexei Starovoitov
2026-04-24 23:20         ` Justin Suess
2026-04-25  1:25           ` Alexei Starovoitov
2026-04-25  2:17             ` Justin Suess [this message]
2026-04-24 19:22 ` [PATCH bpf-next v3 2/2] selftests/bpf: Add test for map-stored struct file kptrs Justin Suess

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=aewkPGl6vkcWS5z9@zenbox \
    --to=utilityemal77@gmail.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=bpf@vger.kernel.org \
    --cc=sashiko@lists.linux.dev \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.