fork: normalize copy_process() error return before ERR_PTR().#1778
fork: normalize copy_process() error return before ERR_PTR().#1778husjdodoanthing wants to merge 1 commit into
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideNormalizes non-negative error codes in copy_process() before returning ERR_PTR() to ensure kernel_clone() always sees proper error pointers and avoid crashes caused by BPF_MODIFY_RETURN hooks returning positive values. Sequence diagram for updated copy_process error handling in fork pathsequenceDiagram
participant kernel_clone
participant copy_process
participant security_task_alloc
participant bpf_program
kernel_clone->>copy_process: copy_process(clone_args)
copy_process->>security_task_alloc: security_task_alloc(task_struct)
security_task_alloc->>bpf_program: BPF_MODIFY_RETURN
bpf_program-->>security_task_alloc: retval (may be > 0)
security_task_alloc-->>copy_process: retval
alt [retval != 0]
note over copy_process: Error path
opt [retval >= 0]
copy_process->>copy_process: retval = -EINVAL
end
copy_process-->>kernel_clone: ERR_PTR(retval)
else [retval == 0]
note over copy_process: Success path (not changed)
end
kernel_clone->>kernel_clone: IS_ERR(copy_process_ret)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Hi @husjdodoanthing. Thanks for your PR. 😃 |
|
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 Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider adding a WARN_ONCE() (or similar) when retval is unexpectedly non-negative so that unexpected BPF or security hook behavior can be surfaced in logs instead of being silently normalized to -EINVAL.
- It might be useful to clarify in the comment why -EINVAL was chosen specifically (vs. propagating a generic -EPERM or -EACCES) to make future readers understand the semantic meaning of the normalized error.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider adding a WARN_ONCE() (or similar) when retval is unexpectedly non-negative so that unexpected BPF or security hook behavior can be surfaced in logs instead of being silently normalized to -EINVAL.
- It might be useful to clarify in the comment why -EINVAL was chosen specifically (vs. propagating a generic -EPERM or -EACCES) to make future readers understand the semantic meaning of the normalized error.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
This PR hardens the copy_process() fork error path by ensuring it never returns a non-error pointer via ERR_PTR(retval) when retval is unexpectedly non-negative (e.g., due to a BPF_MODIFY_RETURN program affecting security_task_alloc()), preventing kernel_clone() from misclassifying the return value and dereferencing an invalid pointer.
Changes:
- Normalize unexpected
retval >= 0to-EINVALimmediately beforereturn ERR_PTR(retval)incopy_process()’s unified error-exit path. - Add an in-code comment explaining why
ERR_PTR(retval)requires a negative errno and what is being prevented.
💡 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>
husjdodoanthing
force-pushed
the
linux-6.6.y
branch
from
84fd69b to
db7755d
Compare
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: