Skip to content

Commit

Permalink
fs-util: rewrite chmod_and_chown()
Browse files Browse the repository at this point in the history
Inspired by #12431 let's also rework chmod_and_chown() and make sure we
never add more rights to a file not owned by the right user.

Also, let's make chmod_and_chown() just a wrapper arond
fchmod_and_chown().

let's also change strategy: instead of chown()ing first and stating
after on failure and supressing errors, let's avoid the chown in the
firts place, in the interest on keeping things minimal.
  • Loading branch information
poettering committed May 24, 2019
1 parent 2570578 commit 2dbb7e9
Showing 1 changed file with 37 additions and 82 deletions.
119 changes: 37 additions & 82 deletions src/basic/fs-util.c
Original file line number Diff line number Diff line change
Expand Up @@ -213,113 +213,68 @@ int readlink_and_make_absolute(const char *p, char **r) {
}

int chmod_and_chown(const char *path, mode_t mode, uid_t uid, gid_t gid) {
char fd_path[STRLEN("/proc/self/fd/") + DECIMAL_STR_MAX(int) + 1];
_cleanup_close_ int fd = -1;
bool st_valid = false;
struct stat st;
int r;

assert(path);

/* Under the assumption that we are running privileged we first change the access mode and only then
* hand out ownership to avoid a window where access is too open. */

fd = open(path, O_PATH|O_CLOEXEC|O_NOFOLLOW); /* Let's acquire an O_PATH fd, as precaution to change
* mode/owner on the same file */
if (fd < 0)
return -errno;

xsprintf(fd_path, "/proc/self/fd/%i", fd);

if (mode != MODE_INVALID) {
if ((mode & S_IFMT) != 0) {

if (stat(fd_path, &st) < 0)
return -errno;

if ((mode & S_IFMT) != (st.st_mode & S_IFMT))
return -EINVAL;

st_valid = true;
}

if (chmod(fd_path, mode & 07777) < 0) {
r = -errno;

if (!st_valid && stat(fd_path, &st) < 0)
return -errno;

if ((mode & 07777) != (st.st_mode & 07777))
return r;

st_valid = true;
}
}

if (uid != UID_INVALID || gid != GID_INVALID) {
if (chown(fd_path, uid, gid) < 0) {
r = -errno;

if (!st_valid && stat(fd_path, &st) < 0)
return -errno;

if (uid != UID_INVALID && st.st_uid != uid)
return r;
if (gid != GID_INVALID && st.st_gid != gid)
return r;
}
}

return 0;
return fchmod_and_chown(fd, mode, uid, gid);
}

int fchmod_and_chown(int fd, mode_t mode, uid_t uid, gid_t gid) {
bool st_valid = false;
char fd_path[STRLEN("/proc/self/fd/") + DECIMAL_STR_MAX(int) + 1];
bool do_chown, do_chmod;
struct stat st;
int r;

/* Under the assumption that we are running privileged we first change the access mode and only then hand out
* ownership to avoid a window where access is too open. */
/* Change ownership and access mode of the specified fd. Tries to do so safely, ensuring that at no
* point in time the access mode is above the old access mode under the old ownership or the new
* access mode under the new ownership. Note: this call tries hard to leave the access mode
* unaffected if the uid/gid is changed, i.e. it undoes implicit suid/sgid dropping the kernel does
* on chown().
*
* This call is happy with O_PATH fds, since we always go via /proc/self/fd/ to change
* ownership/access mode. */

if (mode != MODE_INVALID) {
if ((mode & S_IFMT) != 0) {
xsprintf(fd_path, "/proc/self/fd/%i", fd);
if (stat(fd_path, &st) < 0)
return -errno;

if (fstat(fd, &st) < 0)
return -errno;
do_chown =
(uid != UID_INVALID && st.st_uid != uid) ||
(gid != GID_INVALID && st.st_gid != gid);

if ((mode & S_IFMT) != (st.st_mode & S_IFMT))
return -EINVAL;
do_chmod =
!S_ISLNK(st.st_mode) && /* chmod is not defined on symlinks */
((mode != MODE_INVALID && ((st.st_mode ^ mode) & 07777) != 0) ||
do_chown); /* If we change ownership, make sure we reset the mode afterwards, since chown()
* modifies the access mode too */

st_valid = true;
}
if (mode == MODE_INVALID)
mode = st.st_mode; /* If we only shall do a chown(), save original mode, since chown() might break it. */
else if ((mode & S_IFMT) != 0 && ((mode ^ st.st_mode) & S_IFMT) != 0)
return -EINVAL; /* insist on the right file type if it was specified */

if (fchmod(fd, mode & 07777) < 0) {
r = -errno;
if (do_chown && do_chmod) {
mode_t minimal = st.st_mode & mode; /* the subset of the old and the new mask */

if (!st_valid && fstat(fd, &st) < 0)
if (((minimal ^ st.st_mode) & 07777) != 0)
if (chmod(fd_path, minimal & 07777) < 0)
return -errno;

if ((mode & 07777) != (st.st_mode & 07777))
return r;

st_valid = true;
}
}

if (uid != UID_INVALID || gid != GID_INVALID)
if (fchown(fd, uid, gid) < 0) {
r = -errno;

if (!st_valid && fstat(fd, &st) < 0)
return -errno;
if (do_chown)
if (chown(fd_path, uid, gid) < 0)
return -errno;

if (uid != UID_INVALID && st.st_uid != uid)
return r;
if (gid != GID_INVALID && st.st_gid != gid)
return r;
}
if (do_chmod)
if (chmod(fd_path, mode & 07777) < 0)
return -errno;

return 0;
return do_chown || do_chmod;
}

int fchmod_umask(int fd, mode_t m) {
Expand Down

0 comments on commit 2dbb7e9

Please sign in to comment.