From ff4af908a58eedf9a165946f109f06add23fff9c Mon Sep 17 00:00:00 2001 From: William Hubbs Date: Sun, 2 Dec 2018 16:08:42 -0600 Subject: [PATCH] Revert "checkpath: use O_PATH when available" This reverts commit 2af0cedd5952d7da71681b7a636dff3540e4295d. After speaking with Luis Ressel on the Gentoo selinux team, I am reverting this commit for the following reasons: - Luis told me that he feels this is not the solution we need to address the concern with checkpath; I will be working with him on another solution. - There are concerns about the way the path variable was handled and the assert() call. The path variable should be dynamically allocated using xasprintf instead of defining a length at compile time. This would eliminate the need for the assert() call. - It introduces the definition of _GNU_SOURCE which makes it easier to introduce portability concerns in the future (see #262). --- src/rc/checkpath.c | 44 +++----------------------------------------- 1 file changed, 3 insertions(+), 41 deletions(-) diff --git a/src/rc/checkpath.c b/src/rc/checkpath.c index 62ee120d..448c9cf8 100644 --- a/src/rc/checkpath.c +++ b/src/rc/checkpath.c @@ -16,11 +16,9 @@ * except according to the terms contained in the LICENSE file. */ -#define _GNU_SOURCE #include #include -#include #include #include #include @@ -71,37 +69,6 @@ const char * const longopts_help[] = { }; const char *usagestring = NULL; -/* On Linux, fchmod() returns EBADF when passed a file descriptor opened - * with O_PATH. Use chmod() on /proc/self/fd as a workaround. */ -static int fchmod_opath(int fd, mode_t mode) -{ -#ifdef O_PATH - /* A 64-bit int will result in a maximum path length of 35 characters. */ - char path[35]; - - /* Maybe we will have 128-bit ints someday. */ - assert(sizeof(int) <= 8); - - if (sprintf(path, "/proc/self/fd/%d", fd) < 0) - return -1; - - return chmod(path, mode); -#else - return fchmod(fd, mode); -#endif -} - -/* On Linux, fchown() returns EBADF when passed a file descriptor opened - * with O_PATH. fchownat() does not exhibit this flaw. */ -static int fchown_opath(int fd, uid_t uid, gid_t gid) -{ -#ifdef O_PATH - return fchownat(fd, "", uid, gid, AT_EMPTY_PATH); -#else - return fchown(fd, uid, gid); -#endif -} - static int do_check(char *path, uid_t uid, gid_t gid, mode_t mode, inode_t type, bool trunc, bool chowner, bool selinux_on) { @@ -115,7 +82,7 @@ static int do_check(char *path, uid_t uid, gid_t gid, mode_t mode, memset(&st, 0, sizeof(st)); flags = O_CREAT|O_NDELAY|O_WRONLY|O_NOCTTY; - readflags = O_NDELAY|O_NOCTTY; + readflags = O_NDELAY|O_NOCTTY|O_RDONLY; #ifdef O_CLOEXEC flags |= O_CLOEXEC; readflags |= O_CLOEXEC; @@ -123,11 +90,6 @@ static int do_check(char *path, uid_t uid, gid_t gid, mode_t mode, #ifdef O_NOFOLLOW flags |= O_NOFOLLOW; readflags |= O_NOFOLLOW; -#endif -#ifdef O_PATH - readflags |= O_PATH; -#else - readflags |= O_RDONLY; #endif if (trunc) flags |= O_TRUNC; @@ -215,7 +177,7 @@ static int do_check(char *path, uid_t uid, gid_t gid, mode_t mode, return -1; } einfo("%s: correcting mode", path); - if (fchmod_opath(readfd, mode)) { + if (fchmod(readfd, mode)) { eerror("%s: chmod: %s", applet, strerror(errno)); close(readfd); return -1; @@ -234,7 +196,7 @@ static int do_check(char *path, uid_t uid, gid_t gid, mode_t mode, return -1; } einfo("%s: correcting owner", path); - if (fchown_opath(readfd, uid, gid)) { + if (fchown(readfd, uid, gid)) { eerror("%s: chown: %s", applet, strerror(errno)); close(readfd); return -1;