[Deepin-Kernel-SIG] [linux 6.6-y] [Upstream] uprobes: Fix race in uprobe_free_utask

[opsiff]

mainline inclusion
from mainline-v6.13-rc1
commit b583ef82b671c9a752fbe3e95bd4c1c51eab764d
category: bugfix
bugzilla: https://gitee.com/openeuler/kernel/issues/IB0MX4

Reference: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=b583ef82b671c9a752fbe3e95bd4c1c51eab764d

---

Max Makarov reported kernel panic [1] in perf user callchain code.

The reason for that is the race between uprobe_free_utask and bpf profiler code doing the perf user stack unwind and is triggered within uprobe_free_utask function:

• after current->utask is freed and
• before current->utask is set to NULL

general protection fault, probably for non-canonical address 0x9e759c37ee555c76: 0000 [#1] SMP PTI
RIP: 0010:is_uprobe_at_func_entry+0x28/0x80
...
? die_addr+0x36/0x90
? exc_general_protection+0x217/0x420
? asm_exc_general_protection+0x26/0x30
? is_uprobe_at_func_entry+0x28/0x80
perf_callchain_user+0x20a/0x360
get_perf_callchain+0x147/0x1d0
bpf_get_stackid+0x60/0x90
bpf_prog_9aac297fb833e2f5_do_perf_event+0x434/0x53b
? __smp_call_single_queue+0xad/0x120
bpf_overflow_handler+0x75/0x110
...
asm_sysvec_apic_timer_interrupt+0x1a/0x20
RIP: 0010:__kmem_cache_free+0x1cb/0x350
...
? uprobe_free_utask+0x62/0x80
? acct_collect+0x4c/0x220
uprobe_free_utask+0x62/0x80
mm_release+0x12/0xb0
do_exit+0x26b/0xaa0
__x64_sys_exit+0x1b/0x20
do_syscall_64+0x5a/0x80

It can be easily reproduced by running following commands in separate terminals:

while :; do bpftrace -e 'uprobe:/bin/ls:_start { printf("hit\n"); }' -c ls; done

bpftrace -e 'profile:hz:100000 { @[ustack()] = count(); }'

Fixing this by making sure current->utask pointer is set to NULL before we start to release the utask object.

[1] grafana/pyroscope#3673

Fixes: cfa7f3d2c526 ("perf,x86: avoid missing caller address in stack traces captured in uprobe")
Reported-by: Max Makarov [email protected]

Acked-by: Oleg Nesterov [email protected]
Acked-by: Andrii Nakryiko [email protected]
Link: https://lore.kernel.org/r/[email protected] (cherry picked from commit b583ef82b671c9a752fbe3e95bd4c1c51eab764d) Conflicts:
kernel/events/uprobes.c
[ Context changed in 430af825ba99 ("uprobes: kill the unnecessary put_uprobe/xol_free_insn_slot in uprobe_free_utask() ")]

Summary by Sourcery

Bug Fixes:

• Fixes a race condition in uprobe_free_utask that could lead to kernel panic when perf user callchain code is active, by ensuring current->utask is set to NULL before releasing the utask object.

[sourcery-ai]

Reviewer's Guide by Sourcery

This pull request fixes a race condition in uprobe_free_utask by ensuring that current->utask is set to NULL before the utask object is released. This prevents potential kernel panics when BPF profiler code attempts to unwind the perf user stack after the utask object has been freed but before the pointer is nulled out.

Sequence diagram for uprobe_free_utask fix

sequenceDiagram
  participant Task
  participant utask Object
  Task->>utask Object: utask = current->utask
  alt utask != NULL
    Task->>Task: t->utask = NULL
    Task->>utask Object: put_uprobe(utask->active_uprobe)
    Task->>utask Object: uprobe_clear_state(utask)
    Task->>Task: xol_free_insn_slot(t)
    Task->>utask Object: kfree(utask)
  end

Loading

File-Level Changes

Change	Details	Files
The current->utask pointer is now set to NULL before releasing the utask object.	Moved t->utask = NULL; before freeing the utask object.	kernel/events/uprobes.c

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!
• Generate a plan of action for an issue: Comment @sourcery-ai plan on
  an issue to generate a plan of action for it.

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[deepin-ci-robot]

deepin pr auto review

关键摘要:

• 在uprobe_free_utask函数中，t->utask = NULL;的赋值操作被移动到了函数的开始部分，这可能会影响函数的执行逻辑。如果utask为空，那么t->utask = NULL;的赋值操作是多余的，因为utask本身就是空指针。如果utask不为空，那么t->utask = NULL;的赋值操作是必要的，以确保task_struct结构体中的utask指针被正确地设置为NULL。

是否建议立即修改:

• 是，建议立即修改以保持代码的一致性和逻辑的正确性。如果utask为空，那么应该移除t->utask = NULL;的赋值操作，以避免不必要的操作。如果utask不为空，那么应该保留t->utask = NULL;的赋值操作，以确保task_struct结构体中的utask指针被正确地设置为NULL。

[sourcery-ai]

Hey @opsiff - I've reviewed your changes - here's some feedback:

Overall Comments:

• The commit message is well-written and clearly explains the issue and the fix.

Here's what I looked at during the review

• 🟢 General issues: all looks good
• 🟢 Security: all looks good
• 🟢 Testing: all looks good
• 🟢 Complexity: all looks good
• 🟢 Documentation: all looks good

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[deepin-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: sourcery-ai[bot]
Once this PR has been reviewed and has the lgtm label, please ask for approval from opsiff. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• deepin/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment