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

FreeBSD RESOLVE_BENEATH support and more #541

Merged
merged 4 commits into from
Mar 29, 2023

Conversation

valpackett
Copy link
Contributor

@valpackett valpackett commented Feb 20, 2023

This is everything required to implement support for FreeBSD's (O|AT)_RESOLVE_BENEATH based lookup sandboxing in cap-std :) While here, also expose more Linux-compatible stuff that FreeBSD's libc exposes, also improve kqueue a bit.

The chmodat signature change is an API break :/ but the omission of flags feels like a mistake to me and I think it's better to fix it earlier rather than later. What do you think?

Waiting for: a libc release with rust-lang/libc#3114 rust-lang/libc#3122 rust-lang/libc#3124 wheeee
Includes: #540

@valpackett valpackett force-pushed the freebsd branch 2 times, most recently from bfe0476 to fd6b726 Compare March 13, 2023 06:19
@valpackett valpackett marked this pull request as ready for review March 13, 2023 06:20
@valpackett valpackett force-pushed the freebsd branch 3 times, most recently from 752d5a1 to bd17a8d Compare March 13, 2023 18:06
@valpackett valpackett changed the title FreeBSD RESOLVE_BENEATH support FreeBSD RESOLVE_BENEATH support and more Mar 13, 2023
Copy link
Member

@sunfishcode sunfishcode left a comment

Choose a reason for hiding this comment

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

Cc @notgull in case you're interested in the kqueue changes.

For chmodat, it feels like it might be error-prone to have the flag on Linux where it isn't supported. What would you think about adding a chmodat_with function with a flags argument, that's only available on platforms that support flags?

@valpackett
Copy link
Contributor Author

Hm, that can be done for not breaking backwards compatibility I guess.
But AtFlags is already not something that guarantees that every flag will be used. A lot of the AtFlags are function specific, for example AT_REMOVEDIR is for unlinkat and would do nothing for, say, fstatat. I guess the difference here is the kernel would return EINVAL for unsupported flags, but the ignoring implementation won't—but we can just make it do that.

@sunfishcode
Copy link
Member

Good point, having it fail with EINVAL sounds good. Or maybe ENOTSUP would be better?

The other concern here is that rustix recently had a semver bump with 0.37, and we hopefully won't need another for quite a while. Would you be ok introducing a different name for now, with a plan to change it once we start thinking about the next semver bump?

@valpackett valpackett force-pushed the freebsd branch 5 times, most recently from 4e02ec2 to 475c013 Compare March 20, 2023 05:59
@valpackett
Copy link
Contributor Author

Nah, EINVAL is the correct one for unsupported arguments, that's what all the libcs return in that case.

Fun discovery btw: musl implements AT_SYMLINK_NOFOLLOW there even though glibc doesn't.

Changed to chmodat_with, hopefully the doc stuff looks okay…

src/fs/at.rs Outdated Show resolved Hide resolved
src/backend/libc/fs/syscalls.rs Outdated Show resolved Hide resolved
@valpackett valpackett force-pushed the freebsd branch 3 times, most recently from a4ba539 to 58c2523 Compare March 21, 2023 05:20
@sunfishcode
Copy link
Member

This looks good! I made a note about the naming of chmodat_with in #578.

@sunfishcode sunfishcode merged commit 342c00f into bytecodealliance:main Mar 29, 2023
52 checks passed
@valpackett
Copy link
Contributor Author

Yaay :3 I'll have a little bit more coming today (adding more procctl's right now), hopefully it'd be possible to get that into the next minor release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants