Skip to content

feat(sys): High-level personality(2) wrappers#1331

Merged
bors[bot] merged 1 commit intonix-rust:masterfrom
trailofbits:ww/personality
Dec 19, 2020
Merged

feat(sys): High-level personality(2) wrappers#1331
bors[bot] merged 1 commit intonix-rust:masterfrom
trailofbits:ww/personality

Conversation

@woodruffw
Copy link
Contributor

@woodruffw woodruffw commented Nov 10, 2020

Adds a high level Persona bitflags enum, as well as personality::get() and personality::set() for interacting with libc::personality().

Closes #1330.

See also rust-lang/libc#1974.

woodruffw
woodruffw commented Nov 10, 2020
View reviewed changes
src/sys/personality.rs Outdated

libc_bitflags! {
pub struct Persona: c_ulong {
ADDR_COMPAT_LAYOUT as c_ulong;
Copy link
Contributor Author

@woodruffw woodruffw Nov 10, 2020

Choose a reason for hiding this comment

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

N.B.: These casts won't be necessary if/when rust-lang/libc#1974 lands.

@woodruffw woodruffw mentioned this pull request Nov 10, 2020
Merged
asomers
asomers requested changes Nov 15, 2020
View reviewed changes
Copy link
Member

@asomers asomers left a comment

Choose a reason for hiding this comment

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

I think this would be worthwhile. It just needs a few things:

  • To be rebased once the libc PR lands
  • tests (doc tests are ok)
  • CHANGELOG entry

@woodruffw
Copy link
Contributor Author

woodruffw commented Nov 15, 2020

Sounds good; I can do all of those.

@woodruffw
Copy link
Contributor Author

woodruffw commented Nov 15, 2020

Okay, I've added some doctests and a CHANGELOG entry. I'll also rebase once that PR goes through.

@asomers
Copy link
Member

asomers commented Nov 17, 2020

This looks good. You'll also need to rebase to pick up a Nix fix for those libc deprecations.

@woodruffw
Copy link
Contributor Author

woodruffw commented Nov 18, 2020 •
edited
Loading

Looks like musl libc doesn't support some of these persona values, so I went ahead and cfg'd them out.

Upsteam musl change: https://www.openwall.com/lists/musl/2020/11/18/1

Edit: This was merged. At some point in this distant future, musl users will be able to use these with nix 😉

@woodruffw
Copy link
Contributor Author

woodruffw commented Nov 20, 2020

Last CI failure looks unrelated:

test test_drop ... error: process didn't exit successfully: `/linux-runner x86_64 /target/x86_64-unknown-linux-gnu/release/deps/test_aio_drop-d7c4918b8a06891c` (signal: 1, SIGHUP: hangup)

@asomers
Copy link
Member

asomers commented Dec 3, 2020

Please rebase to fix the CHANGELOG.

@woodruffw
Copy link
Contributor Author

woodruffw commented Dec 7, 2020

I've updated this PR to not be blocked on libc: the signature of personality(2) is c_ulong -> c_int where c_int is always guaranteed to hold a valid persona, so c_int is the correct underlying type.

@asomers
Copy link
Member

asomers commented Dec 8, 2020

I've updated this PR to not be blocked on libc: the signature of personality(2) is c_ulong -> c_int where c_int is always guaranteed to hold a valid persona, so c_int is the correct underlying type.

lol. The C API doesn't make much sense, then.

@asomers
Copy link
Member

asomers commented Dec 8, 2020

This looks good now. It just needs a squash. And you'll probably want to rebase at the same time, or you may get blocked waiting for Travis.

@woodruffw
Copy link
Contributor Author

woodruffw commented Dec 8, 2020

lol. The C API doesn't make much sense, then.

Yep. My educated guess is that it's unsigned long because every syscall parameter is (or was? it's been a moment since I've checked) unsigned long at the end of the day, and whoever wrapped it in glibc et al. got lazy. Oh well.

This looks good now. It just needs a squash. And you'll probably want to rebase at the same time, or you may get blocked waiting for Travis.

👍 I'll squash and rebase tomorrow.

@woodruffw
Copy link
Contributor Author

woodruffw commented Dec 8, 2020

Okay, squashed and rebased. Let me know if there's anything else (&c &c)

@woodruffw
Copy link
Contributor Author

woodruffw commented Dec 9, 2020

Unrelated failure?

failures:

---- src/unistd.rs - unistd::alarm (line 1636) stdout ----
thread 'src/unistd.rs - unistd::alarm (line 1636)' panicked at 'test executable failed:

thread 'main' panicked at 'assertion failed: start.elapsed() >= Duration::from_secs(1)', src/unistd.rs:21:1
note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.

', src/librustdoc/test.rs:341:17
note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.

@asomers
Copy link
Member

asomers commented Dec 15, 2020

Try rebasing again.

@woodruffw
Copy link
Contributor Author

woodruffw commented Dec 15, 2020

Rebased!

asomers
asomers requested changes Dec 15, 2020
View reviewed changes
@woodruffw
feat(sys): High-level personality(2) wrappers
2359b0e 
Adds a high level `Persona` bitflags enum, as well as `personality::get()`
and `personality::set()` for interacting with `libc::personality()`.
@woodruffw
Copy link
Contributor Author

woodruffw commented Dec 15, 2020

(Same unrelated failure as before).

asomers
asomers approved these changes Dec 19, 2020
View reviewed changes
Copy link
Member

@asomers asomers left a comment

Choose a reason for hiding this comment

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

bors r+

@bors
Copy link
Contributor

bors bot commented Dec 19, 2020

Build succeeded:

@bors bors bot merged commit e65db13 into nix-rust:master Dec 19, 2020
@woodruffw woodruffw deleted the ww/personality branch December 19, 2020 18:24
This was referenced Mar 12, 2021
Merged
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@asomers asomers asomers approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Support for personality(2)?

2 participants

@woodruffw @asomers