fork: normalize copy_process() error return before ERR_PTR().#1779
fork: normalize copy_process() error return before ERR_PTR().#1779husjdodoanthing wants to merge 1 commit into
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideNormalize copy_process() error returns to always use a negative errno before wrapping with ERR_PTR(), preventing non-error pointers from being returned to kernel_clone() when BPF hooks return positive values. Sequence diagram for normalized copy_process error returnsequenceDiagram
participant Syscall_fork
participant kernel_clone
participant copy_process
participant security_task_alloc
participant BPF_program
Syscall_fork->>kernel_clone: kernel_clone()
kernel_clone->>copy_process: copy_process()
copy_process->>security_task_alloc: security_task_alloc()
security_task_alloc->>BPF_program: bpf_prog_run()
BPF_program-->>security_task_alloc: retval (may be > 0)
security_task_alloc-->>copy_process: retval
alt retval >= 0
copy_process->>copy_process: [retval >= 0] retval = -EINVAL
end
copy_process-->>kernel_clone: ERR_PTR(retval)
kernel_clone->>kernel_clone: IS_ERR() checks ERR_PTR(retval)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
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 |
There was a problem hiding this comment.
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 visible during debugging rather than silently coerced.
- Since the condition is already rare, the
unlikely()hint aroundretval >= 0may not bring much benefit and slightly hurts readability; consider dropping it unless you’ve measured a hot-path impact here.
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 visible during debugging rather than silently coerced.
- Since the condition is already rare, the `unlikely()` hint around `retval >= 0` may not bring much benefit and slightly hurts readability; consider dropping it unless you’ve measured a hot-path impact here.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() error path in kernel/fork.c to prevent returning non-error pointers via ERR_PTR(retval) when a hook (notably BPF BPF_MODIFY_RETURN on security_task_alloc()) returns an unexpected non-negative value, which can bypass IS_ERR() handling in kernel_clone() and lead to a crash.
Changes:
- Add a guard in the
copy_process()failure exit path to normalize unexpectedretvalvalues beforereturn 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>
2453548 to
899d1d3
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: