All of lore.kernel.org
 help / color / mirror / Atom feed
From: Shakeel Butt <shakeel.butt@linux.dev>
To: sashiko-reviews@lists.linux.dev
Cc: bpf@vger.kernel.org, tj@kernel.org, hannes@cmpxchg.org,
	 mkoutny@suse.com
Subject: Re: [PATCH bpf-next 3/3] bpf, cgroup: Protect cgroup_bpf state with its own mutex
Date: Thu, 12 Nov 2026 10:23:05 -0800	[thread overview]
Message-ID: <avYD5SOXPBQiey5v@linux.dev> (raw)
In-Reply-To: <sashiko-outbox-211618@kernel.org>

On Thu, Nov 12, 2026 at 05:41:57PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found no new issues
> in this patch, but noticed 1 related pre-existing issue it does not
> introduce:
> - [Medium] bpf: lost error codes in __cgroup_bpf_query() [1]
> --
> 
> commit 20190389f8922442abed742617dfa9e59bf26340
> Author: Shakeel Butt <shakeel.butt@linux.dev>
> 
> bpf, cgroup: Protect cgroup_bpf state with its own mutex
> 
> This commit introduces a dedicated cgroup_bpf_mutex to protect cgroup BPF
> state so that operations not modifying the hierarchy do not have to contend
> on cgroup_mutex.
> 
> > diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> > index 334237096759f..ae082ac86130b 100644
> > --- a/kernel/bpf/cgroup.c
> > +++ b/kernel/bpf/cgroup.c
> 
> [ ... ]
> 
> > @@ -1518,7 +1565,7 @@ static int __cgroup_bpf_query(struct cgroup *cgrp, const union bpf_attr *attr,
> >  	for (atype = from_atype; atype <= to_atype && total_cnt; atype++) {
> >  		if (effective_query) {
> >  			effective = rcu_dereference_protected(cgrp->bpf.effective[atype],
> > -							      lockdep_is_held(&cgroup_mutex));
> > +							      cgroup_bpf_held());
> >  			cnt = min_t(int, bpf_cgroup_array_length(effective, atype), total_cnt);
> >  			ret = bpf_cgroup_array_copy_to_user(effective, prog_ids, cnt, atype);
> >  		} else {
> 
> [Severity: Medium]
> This problem wasn't introduced by this patch, but can assigning ret from
> bpf_cgroup_array_copy_to_user() drop an earlier -ENOSPC or -EFAULT error
> [1]?
> 
> Before the loop, __cgroup_bpf_query() sets ret = -ENOSPC when
> attr->query.prog_cnt < total_cnt:
> 
> kernel/bpf/cgroup.c:__cgroup_bpf_query() {
> 	...
> 	if (attr->query.prog_cnt < total_cnt) {
> 		total_cnt = attr->query.prog_cnt;
> 		ret = -ENOSPC;
> 	}
> 
> 	for (atype = from_atype; atype <= to_atype && total_cnt; atype++) {
> 		if (effective_query) {
> 			...
> 			ret = bpf_cgroup_array_copy_to_user(effective, prog_ids, cnt, atype);
> 	...
> }
> 
> When effective_query is set, bpf_cgroup_array_copy_to_user() overwrites
> ret with 0 when the truncated slice fits in the array, or a later
> iteration (for BPF_LSM_CGROUP) can overwrite an earlier -EFAULT, causing
> __cgroup_bpf_query() to return 0 on truncated or faulted queries.
> 
> [1] https://sashiko.dev/#/bug/linux-20442c38-bcc8-440c-9711-1977ea6cd04b

Seems like an independent bug. I will send the fix separately.

      reply	other threads:[~2026-11-12 18:23 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-12 17:27 [PATCH bpf-next 0/3] bpf, cgroup: Don't wait for cgroup_mutex to read or free cgroup bpf state Shakeel Butt
2026-11-12 17:27 ` [PATCH bpf-next 1/3] bpf, cgroup: Don't read freed effective arrays in BPF_PROG_QUERY Shakeel Butt
2026-11-12 17:27 ` [PATCH bpf-next 2/3] bpf, cgroup: Don't attach to a removed cgroup Shakeel Butt
2026-11-12 17:27 ` [PATCH bpf-next 3/3] bpf, cgroup: Protect cgroup_bpf state with its own mutex Shakeel Butt
2026-11-12 17:41   ` sashiko-bot
2026-11-12 18:23     ` Shakeel Butt [this message]

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=avYD5SOXPBQiey5v@linux.dev \
    --to=shakeel.butt@linux.dev \
    --cc=bpf@vger.kernel.org \
    --cc=hannes@cmpxchg.org \
    --cc=mkoutny@suse.com \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=tj@kernel.org \
    /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.