Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix vfork #293

Merged
merged 3 commits into from
Sep 30, 2024
Merged

Fix vfork #293

merged 3 commits into from
Sep 30, 2024

Conversation

jlaitine
Copy link

Summary

Cherry picks from upstream

Impact

Testing

@jlaitine
Copy link
Author

This boots and stays up over weekend. So I suppose we can just merge it?

saluki> uptime
02:29:02 up 2 days, 18:03, load average: 0.00, 0.00, 0.00

pussuw and others added 3 commits September 30, 2024 09:46
There was an error in the fork() routine when system calls are in use:
the child context is saved on the child's user stack, which is incorrect,
the context must be saved on the kernel stack instead.

The result is a full system crash if (when) the child executes on a
different CPU which does not have the same MMU mappings active.
We need to record the parent's integer register context upon exception
entry to a separate non-volatile area. Why?

Because xcp.regs can move due to a context switch within the fork() system
call, be it either via interrupt or a synchronization point.

Fix this by adding a "sregs" area where the saved user context is placed.
The critical section within fork() is also unnecessary.
This commit fixes the regression from apache#13561

In order to determine whether a context switch has occurred,
we can use g_running_task to store the current regs.
This allows us to compare the current register state with the previously
stored state to identify if a context switch has taken place.

Signed-off-by: hujun5 <[email protected]>
@jlaitine jlaitine merged commit bf75048 into master Sep 30, 2024
11 checks passed
@jlaitine jlaitine deleted the fix_vfork branch September 30, 2024 07:35
@pussuw
Copy link

pussuw commented Sep 30, 2024

This is all in upstream so it is totally fine to merge it. Good to hear it's stable now, I was not 100% convinced myself.

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