Page MenuHomeFreeBSD

Allow posix_fadvise in capability mode
ClosedPublic

Authored by emaste on Apr 13 2022, 10:37 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Apr 9, 3:10 PM
Unknown Object (File)
Tue, Apr 9, 10:42 AM
Unknown Object (File)
Tue, Apr 9, 10:37 AM
Unknown Object (File)
Sat, Apr 6, 1:29 PM
Unknown Object (File)
Sat, Apr 6, 11:04 AM
Unknown Object (File)
Feb 10 2024, 5:07 AM
Unknown Object (File)
Feb 7 2024, 6:46 PM
Unknown Object (File)
Dec 20 2023, 10:20 PM
Subscribers

Details

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Noted by Math Ieu in D34761. (Please let me know what name to use in the commit message.)

Noted by Math Ieu in D34761. (Please let me know what name to use in the commit message.)

Just "Mathieu <sigsys@gmail.com>" is fine.

BTW one reason I could think of why setgroups(2) wouldn't be allowed too would be to prevent a process from dropping supplemental groups that could be used for negative permissions. I wonder if there might be something else though...

Some discussion on IRC with @jhb @markj and @brooks about rights for performing posix_fadvise. I note kern_posix_fadvise has a comment /* XXX: CAP_POSIX_FADVISE? */

My feeling is that it's not necessary, but there are lots of bits left and it's straightforward to do so, if we so desire.

My feeling is that it's not necessary, but there are lots of bits left and it's straightforward to do so, if we so desire.

I think I'd still hold off for now. Some operations (like FADV_NORMAL/RANDOM) really don't need any rights, and other operations could perhaps make use of existing rights (CAP_READ for WILLNEED/DONTNEED/NOREUSE maybe?). And maybe we'll end up adding a new operation which really does require a new right that's more specific than a hypothetical CAP_FADVISE.

This revision is now accepted and ready to land.Apr 14 2022, 6:37 PM
This revision was automatically updated to reflect the committed changes.

Huh now that it got mentioned, it's a little bit worrying that it can trigger reads internally (even if the application shouldn't be able to get the data). Could something bad happen from some read side-effects somehow?

Even if it shouldn't be necessary, a CAP_FADVISE could be a good idea. Capsicum rights are so granular already it wouldn't feel excessive. It could also reduce exposure of some kernel complexity you don't need and that's always nice.

My feeling is that it's not necessary, but there are lots of bits left and it's straightforward to do so, if we so desire.

I think I'd still hold off for now. Some operations (like FADV_NORMAL/RANDOM) really don't need any rights, and other operations could perhaps make use of existing rights (CAP_READ for WILLNEED/DONTNEED/NOREUSE maybe?). And maybe we'll end up adding a new operation which really does require a new right that's more specific than a hypothetical CAP_FADVISE.

Makes a lot of sense. Though I suppose even a write-only file could make use of those advices if you wrote random partial blocks and you wanted to try to avoid read-modify-write cycles. Probably not a very realistic use case though (you'd have to know where to write in a file you can't even read...).

Huh now that it got mentioned, it's a little bit worrying that it can trigger reads internally (even if the application shouldn't be able to get the data). Could something bad happen from some read side-effects somehow?

I think the main side effect of fadvise() is that callers can use it to deprioritize already cached file data, making it eligible for reclamation from global LRU if the system is short on free memory. But capsicum is not designed to prevent that kind of problem; you don't need any capabilities at all to allocate memory and put pressure on the global page cache.

Even if it shouldn't be necessary, a CAP_FADVISE could be a good idea. Capsicum rights are so granular already it wouldn't feel excessive. It could also reduce exposure of some kernel complexity you don't need and that's always nice.

I'm reluctant to add CAP_FADVISE largely because it's not granular enough, per the discussion elsewhere in this review.

My feeling is that it's not necessary, but there are lots of bits left and it's straightforward to do so, if we so desire.

I think I'd still hold off for now. Some operations (like FADV_NORMAL/RANDOM) really don't need any rights, and other operations could perhaps make use of existing rights (CAP_READ for WILLNEED/DONTNEED/NOREUSE maybe?). And maybe we'll end up adding a new operation which really does require a new right that's more specific than a hypothetical CAP_FADVISE.

Makes a lot of sense. Though I suppose even a write-only file could make use of those advices if you wrote random partial blocks and you wanted to try to avoid read-modify-write cycles. Probably not a very realistic use case though (you'd have to know where to write in a file you can't even read...).

Yeah, I don't think we have a clean way to express that either CAP_READ or CAP_WRITE is sufficient when looking up an FD, but I'd be inclined to require at least one of the two for WILLNEED/DONTNEED/NOREUSE. Even though they won't directly cause the system to issue disk I/O, they can, e.g., trigger NFSPROC_IOADVISE RPCs from the NFS client. And jhb pointed out elsewhere that posix_fadvise(DONTNEED/NOREUSE) have side effects similar to those of reading or writing a file opened with O_DIRECT.