trifectatechfoundation / sudo-rs

A memory safe implementation of sudo and su.
Other
2.87k stars 76 forks source link

Make `sigaction` handling more portable #830

Closed squell closed 6 months ago

squell commented 6 months ago

Closing #829.

Note that system/signal/set.rs has a light-weigth SignalAction wrapper, which it probably would be wise to use in term/user_term.rs, but that would be a bigger change and (given that that latter routine is more or less a straight port of original sudo code), this PR is a conservative update to avoid breaking things.

codecov[bot] commented 6 months ago

Codecov Report

Attention: Patch coverage is 0% with 29 lines in your changes are missing coverage. Please review.

Project coverage is 58.65%. Comparing base (4fa3217) to head (a6ad4ee).

Files Patch % Lines
src/system/term/user_term.rs 0.00% 16 Missing :warning:
src/system/signal/set.rs 0.00% 7 Missing :warning:
src/system/mod.rs 0.00% 6 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #830 +/- ## ========================================== - Coverage 58.69% 58.65% -0.04% ========================================== Files 75 75 Lines 10791 10800 +9 ========================================== + Hits 6334 6335 +1 - Misses 4457 4465 +8 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

decathorpe commented 6 months ago

This only addresses one of the occurrences of the error in user_term.rs, but not the same issue further up in the same file:

error: cannot construct `sigaction` with struct literal syntax due to private fields
  --> src/system/term/user_term.rs:67:18
   |
67 |     let action = sigaction {
   |                  ^^^^^^^^^
   |
   = note: ...and other private field `__glibc_reserved0` that was not provided
error: could not compile `sudo-rs` (lib) due to 1 previous error

But it looks like the proposed change would indeed fix the issues on s390x (if applied in all necessary places). :)

squell commented 6 months ago

Didn't catch that there was a third instance of the same thing. Let me fix that.

squell commented 6 months ago

I think this now squashes all instances of sigaction being directly initialised; if that solves the issue we can go ahead and merge it.

decathorpe commented 6 months ago

Can confirm that this now builds successfully on all architectures that I have access to.

decathorpe commented 6 months ago

Thank you!