emo/openrc
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Revert "checkpath: use O_PATH when available"

Browse Source 
This reverts commit 2af0cedd59.

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).
This commit is contained in:
William Hubbs 2018-12-02 16:08:42 -06:00
parent 2af0cedd59
commit ff4af908a5
1 changed files with 3 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
src/rc/checkpath.c
View File

@ -16,11 +16,9 @@
* except according to the terms contained in the LICENSE file.
*/
#define _GNU_SOURCE
#include <sys/types.h>
#include <sys/stat.h>
#include <assert.h>
#include <errno.h>
#include <fcntl.h>
#include <getopt.h>
@ -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;