Skip to content

fork: normalize copy_process() error return before ERR_PTR().#1780

Open
husjdodoanthing wants to merge 1 commit into
deepin-community:linux-6.18.yfrom
husjdodoanthing:linux-6.18.y
Open

fork: normalize copy_process() error return before ERR_PTR().#1780
husjdodoanthing wants to merge 1 commit into
deepin-community:linux-6.18.yfrom
husjdodoanthing:linux-6.18.y

Conversation

@husjdodoanthing
Copy link
Copy Markdown

@husjdodoanthing husjdodoanthing commented May 29, 2026

copy_process() returns ERR_PTR(retval) from its error path, so retval must be a negative errno. If retval is zero or positive, ERR_PTR(retval) produces a non-error pointer that is not caught by IS_ERR() in kernel_clone().

A BPF_MODIFY_RETURN program attached to security_task_alloc() can return a positive value. copy_process() treats the non-zero return as a failure and then returns ERR_PTR(1). kernel_clone() does not treat that as an error and later dereferences the pointer, causing a kernel crash.

Normalize unexpected non-negative values before returning ERR_PTR() from copy_process(). This keeps the fix local to the fork error path and does not change BPF_MODIFY_RETURN verifier behavior.

The issue has been reported and discussed upstream, but the verifier-side fix attempt has not been accepted. Carry this targeted fix in deepin-kernel to prevent the reported denial-of-service.

Link: https://lore.kernel.org/bpf/973a1b7b-8ee7-407a-890a-11455d9cc5bf@std.uestc.edu.cn/
Link: https://lore.kernel.org/all/20260411163556.8567-1-yangfeng59949@163.com/
Reported-by: Quan Sun 2022090917019@std.uestc.edu.cn
Reported-by: Yinhao Hu dddddd@hust.edu.cn
Reported-by: Kaiyan Mei M202472210@hust.edu.cn

Summary by Sourcery

Bug Fixes:

  • Guard against non-negative retval values in copy_process() by mapping them to -EINVAL before returning ERR_PTR() to avoid kernel_clone() dereferencing invalid task_struct pointers.

@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented May 29, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Normalizes the error return value in copy_process() before wrapping it in ERR_PTR(), ensuring only negative errno codes are returned to callers to prevent non-error pointers from propagating and causing kernel crashes when BPF hooks return positive values.

Sequence diagram for copy_process() error normalization during fork

sequenceDiagram
    participant User
    participant kernel_clone
    participant copy_process
    participant security_task_alloc
    participant BPF_MODIFY_RETURN_prog

    User->>kernel_clone: sys_clone()
    kernel_clone->>copy_process: copy_process()
    copy_process->>security_task_alloc: security_task_alloc()
    security_task_alloc->>BPF_MODIFY_RETURN_prog: BPF hook
    BPF_MODIFY_RETURN_prog-->>security_task_alloc: retval (may be > 0)
    security_task_alloc-->>copy_process: retval

    alt [retval < 0]
        copy_process-->>kernel_clone: ERR_PTR(retval)
        kernel_clone->>kernel_clone: IS_ERR() detects error
    else [retval >= 0]
        copy_process->>copy_process: normalize retval to -EINVAL
        copy_process-->>kernel_clone: ERR_PTR(-EINVAL)
        kernel_clone->>kernel_clone: IS_ERR() detects error
    end
Loading

File-Level Changes

Change Details Files
Normalize non-negative retval in copy_process() error path before returning via ERR_PTR().
  • Add a sanity check on retval in the error path of copy_process() to detect unexpected non-negative values.
  • Convert any non-negative retval to -EINVAL to ensure ERR_PTR() always receives a negative errno.
  • Document via comments why normalization is needed, referencing the requirement for negative errno values and the risk of returning non-error pointers.
kernel/fork.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!

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

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign avenger-285714 for approval. For more information see the Code Review Process.

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

Details Needs approval from an approver in each of these files:

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

@deepin-ci-robot
Copy link
Copy Markdown

Hi @husjdodoanthing. Thanks for your PR.

I'm waiting for a deepin-community member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've left some high level feedback:

  • Consider adding a WARN_ON_ONCE(retval >= 0) before normalizing to -EINVAL so that unexpected non-negative paths are surfaced during development while still returning a proper ERR_PTR to callers.
  • It may be worth clarifying in the new comment that this normalization specifically covers unexpected positive returns from LSM/BPF hooks like security_task_alloc(), to make the rationale clearer for future maintainers.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Consider adding a WARN_ON_ONCE(retval >= 0) before normalizing to -EINVAL so that unexpected non-negative paths are surfaced during development while still returning a proper ERR_PTR to callers.
- It may be worth clarifying in the new comment that this normalization specifically covers unexpected positive returns from LSM/BPF hooks like security_task_alloc(), to make the rationale clearer for future maintainers.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Normalizes the retval in copy_process()'s error path so that a non-negative value (potentially produced by a BPF_MODIFY_RETURN program attached to security_task_alloc()) cannot be passed to ERR_PTR(), preventing kernel_clone() from dereferencing a non-error pointer and crashing.

Changes:

  • Add guard in copy_process() to coerce non-negative retval to -EINVAL before ERR_PTR(retval).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

copy_process() returns ERR_PTR(retval) from its error path, so retval
must be a negative errno. If retval is zero or positive, ERR_PTR(retval)
produces a non-error pointer that is not caught by IS_ERR() in
kernel_clone().

A BPF_MODIFY_RETURN program attached to security_task_alloc() can return
a positive value. copy_process() treats the non-zero return as a failure
and then returns ERR_PTR(1). kernel_clone() does not treat that as an
error and later dereferences the pointer, causing a kernel crash.

Normalize unexpected non-negative values before returning ERR_PTR() from
copy_process(). This keeps the fix local to the fork error path and does
not change BPF_MODIFY_RETURN verifier behavior.

The issue has been reported and discussed upstream, but the verifier-side
fix attempt has not been accepted. Carry this targeted fix in deepin-kernel
to prevent the reported denial-of-service.

Link: https://lore.kernel.org/bpf/973a1b7b-8ee7-407a-890a-11455d9cc5bf@std.uestc.edu.cn/
Link: https://lore.kernel.org/all/20260411163556.8567-1-yangfeng59949@163.com/
Reported-by: Quan Sun <2022090917019@std.uestc.edu.cn>
Reported-by: Yinhao Hu <dddddd@hust.edu.cn>
Reported-by: Kaiyan Mei <M202472210@hust.edu.cn>
Signed-off-by: hushijia <hushijia1@uniontech.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants