All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jiri Olsa <olsajiri@gmail.com>
To: sashiko-reviews@lists.linux.dev
Cc: bpf@vger.kernel.org
Subject: Re: [PATCHv2 bpf-next 1/6] bpf: Use user_path_at for path resolution in uprobe_multi
Date: Thu, 11 Jun 2026 11:37:37 +0200	[thread overview]
Message-ID: <aiqB4eSTxR4Aw9YJ@krava> (raw)
In-Reply-To: <20260610145235.CB1441F00893@smtp.kernel.org>

On Wed, Jun 10, 2026 at 02:52:35PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [Critical] Missing `access_ok()` checks before calling `__get_user()` on user-controlled arrays, leading to arbitrary kernel memory read.
> --
> 
> commit e18574361f20d4f27005e4d927610e3ce16986dd
> Author: Jiri Olsa <jolsa@kernel.org>
> 
> bpf: Use user_path_at for path resolution in uprobe_multi
> 
> Resolve the uprobe_multi user path with user_path_at() instead of copying
> the string with strndup_user() and passing it to kern_path(). This removes
> the temporary allocation and keeps the lookup logic in one helper.
> 
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index 90432f0fc2a8e..970ce7bbf99e1 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> 
> [ ... ]
> 
> > @@ -3261,14 +3260,7 @@ int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
> >  	uref_ctr_offsets = u64_to_user_ptr(attr->link_create.uprobe_multi.ref_ctr_offsets);
> >  	ucookies = u64_to_user_ptr(attr->link_create.uprobe_multi.cookies);
> >  
> > -	name = strndup_user(upath, PATH_MAX);
> > -	if (IS_ERR(name)) {
> > -		err = PTR_ERR(name);
> > -		return err;
> > -	}
> > -
> > -	err = kern_path(name, LOOKUP_FOLLOW, &path);
> > -	kfree(name);
> > +	err = user_path_at(AT_FDCWD, upath, LOOKUP_FOLLOW, &path);
> >  	if (err)
> >  		return err;
> >  
> 
> [Severity: Critical]
> This isn't a bug introduced by this patch, but while looking at
> bpf_uprobe_multi_link_attach(), I noticed the user-provided arrays
> uoffsets, uref_ctr_offsets, and ucookies are accessed shortly after this
> path resolution:
> 
> kernel/trace/bpf_trace.c:bpf_uprobe_multi_link_attach() {
>     ...
>     if (__get_user(uprobes[i].offset, uoffsets + i)) {
>     ...
>     if (uref_ctr_offsets && __get_user(uprobes[i].ref_ctr_offset, uref_ctr_offsets + i)) {
>     ...
>     if (ucookies && __get_user(uprobes[i].cookie, ucookies + i)) {
>     ...
> }
> 
> Are these arrays validated with access_ok() before __get_user() is called?
> 
> Because __get_user() bypasses the access_ok() address-space bounds check
> on most architectures, could an attacker supply kernel addresses for these
> arrays, causing the kernel to read arbitrary memory and store it into the
> uprobes structures?
> 
> If an attacker retrieves these contents by querying the link info via
> bpf_obj_get_info_by_fd() (which calls bpf_uprobe_multi_link_fill_link_info()
> and copies the memory back to user space), could this act as an arbitrary
> kernel memory read primitive to bypass kernel lockdown and KASLR?

ugh right.. will include fix for this in next version

jirka

  reply	other threads:[~2026-06-11  9:37 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-10 14:36 [PATCHv2 bpf-next 0/6] bpf: Allow uprobe_multi binary specified by file descriptor Jiri Olsa
2026-06-10 14:36 ` [PATCHv2 bpf-next 1/6] bpf: Use user_path_at for path resolution in uprobe_multi Jiri Olsa
2026-06-10 14:52   ` sashiko-bot
2026-06-11  9:37     ` Jiri Olsa [this message]
2026-06-10 14:36 ` [PATCHv2 bpf-next 2/6] bpf: Add support to specify uprobe_multi target via file descriptor Jiri Olsa
2026-06-10 15:51   ` bot+bpf-ci
2026-06-11  9:37     ` Jiri Olsa
2026-06-10 14:36 ` [PATCHv2 bpf-next 3/6] libbpf: Add path_fd to struct bpf_link_create_opts Jiri Olsa
2026-06-10 14:50   ` sashiko-bot
2026-06-11  9:37     ` Jiri Olsa
2026-06-10 14:36 ` [PATCHv2 bpf-next 4/6] selftests/bpf: Add uprobe_multi path_fd test Jiri Olsa
2026-06-10 14:50   ` sashiko-bot
2026-06-11  9:37     ` Jiri Olsa
2026-06-10 14:36 ` [PATCHv2 bpf-next 5/6] selftests/bpf: Add uprobe_multi path_fd fail tests Jiri Olsa
2026-06-10 14:48   ` sashiko-bot
2026-06-11  9:37     ` Jiri Olsa
2026-06-10 14:36 ` [PATCHv2 bpf-next 6/6] selftests/bpf: Fix typo in verify_umulti_link_info Jiri Olsa

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=aiqB4eSTxR4Aw9YJ@krava \
    --to=olsajiri@gmail.com \
    --cc=bpf@vger.kernel.org \
    --cc=sashiko-reviews@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.