BPF List
 help / color / mirror / Atom feed
From: Waiman Long <longman@redhat.com>
To: Markus Elfring <Markus.Elfring@web.de>,
	Chen Ridong <chenridong@huawei.com>,
	cgroups@vger.kernel.org, bpf@vger.kernel.org,
	Johannes Weiner <hannes@cmpxchg.org>, Tejun Heo <tj@kernel.org>,
	Zefan Li <lizefan.x@bytedance.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Julia Lawall <julia.lawall@inria.fr>,
	Peter Zijlstra <peterz@infradead.org>
Subject: Re: [PATCH] cgroup/cpuset: Prevent UAF in proc_cpuset_show()
Date: Sun, 23 Jun 2024 12:28:21 -0400	[thread overview]
Message-ID: <dc0b2650-4c0d-4f52-8c60-b9afbe728103@redhat.com> (raw)
In-Reply-To: <f40c4a72-0c6c-4846-a926-ba1eb2763697@web.de>


On 6/23/24 02:18, Markus Elfring wrote:
>>> …
>>>> +++ b/kernel/cgroup/cpuset.c
>>> …
>>>> @@ -5051,10 +5066,12 @@ int proc_cpuset_show(struct seq_file *m, struct pid_namespace *ns,
>>>>        if (!buf)
>>>>            goto out;
>>>>
>>>> +    mutex_lock(&cpuset_mutex);
>>>>        css = task_get_css(tsk, cpuset_cgrp_id);
>>>>        retval = cgroup_path_ns(css->cgroup, buf, PATH_MAX,
>>>>                    current->nsproxy->cgroup_ns);
>>>>        css_put(css);
>>>> +    mutex_unlock(&cpuset_mutex);
>>> …
>>>
>>> Under which circumstances would you become interested to apply a statement
>>> like “guard(mutex)(&cpuset_mutex);”?
>>> https://elixir.bootlin.com/linux/v6.10-rc4/source/include/linux/mutex.h#L196
>> A mutex guard will be more appropriate if there is an error exit case that needs to be handled.
> Lock guards can help to reduce and improve source code another bit,
> can't they?

For simple lock critical section, there isn't too much difference in 
term of readability between using lock guard and normal lock/unlock 
call. If there are multiple exit points in the critical section, lock 
guard can help to simplify the code. For those situations, I will 
certain try to use lock guard.

Another reason that I go with normal lock/unlock is that none of the 
cpuset_mutex lock/unlock sites in cpuset.c has used lock guard yet and 
there is no good reason in introduce something different from other call 
sites.

Cheers,
Longman



  reply	other threads:[~2024-06-23 16:28 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-22 11:38 [PATCH -next] cgroup: fix uaf when proc_cpuset_show Chen Ridong
2024-06-22 13:45 ` Markus Elfring
2024-06-24  3:34   ` chenridong
2024-06-22 15:05 ` Waiman Long
2024-06-22 20:04   ` [PATCH] cgroup/cpuset: Prevent UAF in proc_cpuset_show() Markus Elfring
2024-06-22 20:12     ` Waiman Long
2024-06-23  6:18       ` Markus Elfring
2024-06-23 16:28         ` Waiman Long [this message]
2024-06-24  2:59   ` [PATCH -next] cgroup: fix uaf when proc_cpuset_show chenridong
2024-06-24 23:59     ` Waiman Long
2024-06-25  1:46       ` chenridong
2024-06-25  2:40         ` Waiman Long
2024-06-25  3:12           ` chenridong
2024-06-25 10:10             ` Michal Koutný
     [not found]               ` <920bbfaa-bb76-4aa1-bd07-9a552e3bfdf2@huawei.com>
2024-06-25 14:16                 ` Waiman Long
2024-06-25 14:29                   ` chenridong

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=dc0b2650-4c0d-4f52-8c60-b9afbe728103@redhat.com \
    --to=longman@redhat.com \
    --cc=Markus.Elfring@web.de \
    --cc=bpf@vger.kernel.org \
    --cc=cgroups@vger.kernel.org \
    --cc=chenridong@huawei.com \
    --cc=hannes@cmpxchg.org \
    --cc=julia.lawall@inria.fr \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lizefan.x@bytedance.com \
    --cc=peterz@infradead.org \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox