All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yonghong Song <yhs@fb.com>
To: Lee Jones <lee@kernel.org>
Cc: linux-kernel@vger.kernel.org, Jiri Olsa <jolsa@kernel.org>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	John Fastabend <john.fastabend@gmail.com>,
	Andrii Nakryiko <andrii@kernel.org>,
	Martin KaFai Lau <martin.lau@linux.dev>,
	Song Liu <song@kernel.org>, KP Singh <kpsingh@kernel.org>,
	Stanislav Fomichev <sdf@google.com>, Hao Luo <haoluo@google.com>,
	bpf@vger.kernel.org
Subject: Re: [PATCH v3 1/1] bpf: Ensure correct locking around vulnerable function find_vpid()
Date: Mon, 12 Sep 2022 17:02:22 +0100	[thread overview]
Message-ID: <a6e56396-f1f9-d939-cc25-201db242e3b8@fb.com> (raw)
In-Reply-To: <Yx9Mv4ltoNhuU0Om@google.com>



On 9/12/22 4:14 PM, Lee Jones wrote:
> On Mon, 12 Sep 2022, Yonghong Song wrote:
>> On 9/12/22 2:38 PM, Lee Jones wrote:
>>> The documentation for find_vpid() clearly states:
>>>
>>
>> No need for the above extra line.
> 
> The intentional blank formatting line?
> 
> The commit message would look worse without it.
> 
> Is this really a blocker?

Not a blocker. Just a suggestion. I won't enforce it
if you do have this extra line.

> 
>>>     "Must be called with the tasklist_lock or rcu_read_lock() held."
>>>
>>> Presently we do neither.
>>
>> Presently we do neither for find_vpid() instance in bpf_task_fd_query().
>> Add proper rcu_read_lock/unlock() to fix the issue.
> 
> I can expand this.
> 
>>> Cc: Jiri Olsa <jolsa@kernel.org>
>>> Cc: Alexei Starovoitov <ast@kernel.org>
>>> Cc: Daniel Borkmann <daniel@iogearbox.net>
>>> Cc: John Fastabend <john.fastabend@gmail.com>
>>> Cc: Andrii Nakryiko <andrii@kernel.org>
>>> Cc: Martin KaFai Lau <martin.lau@linux.dev>
>>> Cc: Song Liu <song@kernel.org>
>>> Cc: Yonghong Song <yhs@fb.com>
>>> Cc: KP Singh <kpsingh@kernel.org>
>>> Cc: Stanislav Fomichev <sdf@google.com>
>>> Cc: Hao Luo <haoluo@google.com>
>>> Cc: bpf@vger.kernel.org
>>> Fixes: 41bdc4b40ed6f ("bpf: introduce bpf subcommand BPF_TASK_FD_QUERY")
>>> Signed-off-by: Lee Jones <lee@kernel.org>
>>
>> Ack with above a few suggestions for the commit message.
>>
>> Acked-by: Yonghong Song <yhs@fb.com>
> 

  reply	other threads:[~2022-09-12 16:02 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-12 13:38 [PATCH v3 1/1] bpf: Ensure correct locking around vulnerable function find_vpid() Lee Jones
2022-09-12 14:55 ` Yonghong Song
2022-09-12 15:14   ` Lee Jones
2022-09-12 16:02     ` Yonghong Song [this message]
2022-09-16 16:10 ` 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=a6e56396-f1f9-d939-cc25-201db242e3b8@fb.com \
    --to=yhs@fb.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=haoluo@google.com \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kpsingh@kernel.org \
    --cc=lee@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=sdf@google.com \
    --cc=song@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.