checkpath: use O_PATH when available #244
Conversation
floppym
force-pushed
the
checkpath-o-path
branch
from
2ce0011
to
80b54e0
Compare
src/rc/checkpath.c
Outdated
| static int fchmod_opath(int fd, mode_t mode) | ||
| { | ||
| #ifdef O_PATH | ||
| char path[50]; |
There was a problem hiding this comment.
path[50] seems arbitrary, why not something like https://github.com/systemd/systemd/blob/master/src/tmpfiles/tmpfiles.c (e.g. DECIMAL_STR_MAX() macro)?
There was a problem hiding this comment.
I did not want to directly copy/paste code from systemd for copyright reasons.
There was a problem hiding this comment.
It appears the number of FDs is limited to unsigned long in the kernel, so the max the path could be is 24 chars (then 25 with the NULL). so its fine as is yeah
There was a problem hiding this comment.
It looks like the posix version of this, which Linux doesn't implement, is O_SEARCH, but I do not have a way to test [1] [2].
Instead of using a hard-coded array of characters for the paths, I would include helpers.h and use xasprintf() to create them.
[1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html
[2] https://stackoverflow.com/questions/44159325/why-is-there-no-o-search-flag-under-linux-for-open-function
There was a problem hiding this comment.
We know the size of the string cannot exceed 26 bytes on a system with 32-bit ints, or 35 bytes if ints are 64-bits. 50 bytes was just a ballpark overestimate.
A fixed-size buffer allocated on the stack is more efficient than calling malloc and free.
There was a problem hiding this comment.
sounds reasonable. can you add a short comment above the static buffer explaining ?
floppym
force-pushed
the
checkpath-o-path
branch
from
80b54e0
to
b45a554
Compare
|
I pushed a new version that uses |
src/rc/checkpath.c
Outdated
| static int fchmod_opath(int fd, mode_t mode) | ||
| { | ||
| #ifdef O_PATH | ||
| char path[50]; |
There was a problem hiding this comment.
sounds reasonable. can you add a short comment above the static buffer explaining ?
src/rc/checkpath.c
Outdated
| @@ -69,6 +70,37 @@ const char * const longopts_help[] = { | |||
| }; | |||
| const char *usagestring = NULL; | |||
|
|
|||
| /* | |||
| * Work around limitation in Linux when calling fchmod() on a file | |||
There was a problem hiding this comment.
can you add a bit more detail (and perhaps include a link to man7.org) here ? i know what limitation you're referring to, but for people who aren't into these low level esoteric details, this comment is just mysterious.
This avoids opening directories/files with read permission, which is sometimes rejected by selinux policy. Bug: https://bugs.gentoo.org/667122
floppym
force-pushed
the
checkpath-o-path
branch
from
b45a554
to
734dc05
Compare
|
New version pushed with more detailed comments. |
|
thanks! |
This avoids opening directories/files with read permission, which is
sometimes rejected by selinux policy.
Bug: https://bugs.gentoo.org/667122