fix: correct fcntl signal owner handling#1876
fix: correct fcntl signal owner handling#1876flynojy wants to merge 1 commit intoDragonOS-Community:masterfrom
Conversation
|
this is also my account. I accidentally used the wrong one to open this merge request |
| int main(int argc, char** argv) { | ||
| ::testing::InitGoogleTest(&argc, argv); | ||
| return RUN_ALL_TESTS(); | ||
| } No newline at end of file |
There was a problem hiding this comment.
No new lines at the file end?
| close(fd); | ||
| } | ||
|
|
||
| // F_SETSIG 后 F_GETSIG 应返回同一值。 |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7278ed9114
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .read() | ||
| .get_file_by_fd(fd) | ||
| .ok_or(SystemError::EBADF)?; | ||
| file.set_fasync_signum(arg as i32); |
There was a problem hiding this comment.
Use configured F_SETSIG value for async I/O delivery
This branch records the requested signal with file.set_fasync_signum(...), but the notification path still always emits SIGIO (FAsyncItems::send_sigio_to_process hard-codes Signal::SIGIO_OR_POLL), so fcntl(fd, F_SETSIG, <nonzero>) never changes the delivered signal. In workloads that rely on O_ASYNC plus a real-time/alternate signal, the new API returns success but behaves incorrectly versus Linux semantics.
Useful? React with 👍 / 👎.
| if arg > Signal::SIGRTMAX as usize { | ||
| return Err(SystemError::EINVAL); |
There was a problem hiding this comment.
Validate F_SETSIG after narrowing arg to int
The new check compares the full usize syscall argument against SIGRTMAX, but Linux F_SETSIG semantics treat the third argument as an int (high bits are truncated for raw syscall callers). As written, values whose low 32 bits are valid signals (e.g. 0x1_0000_0001) are rejected with EINVAL here, creating Linux 6.6 ABI incompatibility for non-libc/compat callers.
Useful? React with 👍 / 👎.
Summary
Fix fcntl signal owner handling and add dunitest coverage for the related behavior.
Changes
fcntl_signaldunitest case.Testing
user/apps/tests/dunitest/suites/normal/fcntl_signal.cc.