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

Idea on how to fix: fork syscall bug #2671

Merged
merged 3 commits into from
May 20, 2023
Merged

Conversation

robertmin1
Copy link
Contributor

@robertmin1 robertmin1 commented May 2, 2023

Adding unix.PTRACE_O_TRACEEXEC to the Ptrace Options fixes the fork issue. This will enable us to avoid seeing the SIGTRAP signal. This option tells ptrace to generate a SIGTRAP signal immediately before a new program is executed with the execve system call.

Fixes: #2670 #2635 and #2590

Signed-off-by: Robert Mindo mindo.robert1@gmail.com

@robertmin1 robertmin1 changed the title Idea on how to fix: #2670 #2635 and #2590 Idea on how to fix: fork syscall bug May 2, 2023
@rminnich rminnich added the Awaiting author Waiting for new changes or feedback for author. label May 2, 2023
@JeremyRand
Copy link

Concept ACK, but would be nice to add the execve test case to the tests, so that we don't risk a regression in the future.

Copy link
Member

@rminnich rminnich left a comment

Choose a reason for hiding this comment

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

nice catch! Please sign off and we can get it in!

@rminnich
Copy link
Member

rminnich commented May 2, 2023

yes, please add a test.

@robertmin1
Copy link
Contributor Author

Alright, I will work on that

Signed-off-by: robertmin1 <104002271+robertmin1@users.noreply.github.com>
@robertmin1 robertmin1 reopened this May 19, 2023
@robertmin1
Copy link
Contributor Author

robertmin1 commented May 19, 2023

Since the current fork.c is similar to our test file, we can maybe replace it, instead of implementing another test.

@robertmin1 robertmin1 added Awaiting reviewer Waiting for a reviewer. and removed Awaiting author Waiting for new changes or feedback for author. labels May 19, 2023
@codecov
Copy link

codecov bot commented May 19, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.05 🎉

Comparison is base (cfd9380) 75.43% compared to head (5f6b9a2) 75.49%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2671      +/-   ##
==========================================
+ Coverage   75.43%   75.49%   +0.05%     
==========================================
  Files         413      413              
  Lines       41963    41964       +1     
==========================================
+ Hits        31654    31679      +25     
+ Misses      10309    10285      -24     
Impacted Files Coverage Δ
pkg/strace/tracer.go 77.20% <100.00%> (+8.05%) ⬆️

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@rminnich rminnich added automerge Applying this label auto-merges the PR when ready Awaiting author Waiting for new changes or feedback for author. and removed Awaiting reviewer Waiting for a reviewer. labels May 19, 2023
@robertmin1 robertmin1 merged commit 5b74f07 into u-root:main May 20, 2023
11 checks passed
@robertmin1 robertmin1 deleted the fork_bug branch May 24, 2023 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Applying this label auto-merges the PR when ready Awaiting author Waiting for new changes or feedback for author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The child process of a fork gets a SIGTRAP
3 participants