From a7ac562f9fbb22663a50708d0fff99cc66b18b2f Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Wed, 29 May 2024 16:11:00 +0900 Subject: [PATCH 1/2] main: Fix file owner retrieval for chmod do_fchmod () and do_chmod () used to call override_mode () directly to retrieve the owner information, but the usage of override_mode () was wrong; override_mode () expects struct stat is already populated by the information provided by the underlying filesystem, but do_fchmod () and do_chmod () only zeroed st_uid and st_gid. override_mode () does not update the owner information when st_mode is not S_IFDIR nor S_IFREG so this caused chmod to change the file owner to root at random. Use the logic rpl_stat () employs to file owner retrieval for chmod functions to ensure they provide the owner information consistent with rpl_stat (). Signed-off-by: Akihiko Odaki --- main.c | 53 +++++++++++++++++++++++++---------------------------- 1 file changed, 25 insertions(+), 28 deletions(-) diff --git a/main.c b/main.c index 3a49a1b..fc5cc79 100644 --- a/main.c +++ b/main.c @@ -622,22 +622,32 @@ do_fchownat (struct ovl_data *lo, int dfd, const char *path, uid_t uid, gid_t gi #define fchownat ERROR static int -do_fchmod (struct ovl_data *lo, int fd, mode_t mode) +do_stat (struct ovl_node *node, int fd, const char *path, struct stat *st) +{ + struct ovl_layer *l = node->layer; + + if (fd >= 0) + return l->ds->fstat (l, fd, path, STATX_BASIC_STATS, st); + + if (path != NULL) + return stat (path, st); + + if (node->hidden) + return fstatat (node_dirfd (node), node->path, st, AT_SYMLINK_NOFOLLOW); + + return l->ds->statat (l, node->path, st, AT_SYMLINK_NOFOLLOW, STATX_BASIC_STATS); +} + +static int +do_fchmod (struct ovl_data *lo, struct ovl_node *node, int fd, mode_t mode) { if (lo->xattr_permissions) { - struct ovl_layer *upper = get_upper_layer (lo); struct stat st; - if (upper == NULL) - { - errno = EROFS; - return -1; - } - st.st_uid = 0; st.st_gid = 0; - if (override_mode (upper, fd, NULL, NULL, &st) < 0 && errno != ENODATA) + if (do_stat (node, fd, NULL, &st) < 0) return -1; return write_permission_xattr (lo, fd, NULL, st.st_uid, st.st_gid, mode); @@ -648,22 +658,15 @@ do_fchmod (struct ovl_data *lo, int fd, mode_t mode) #define fchmod ERROR static int -do_chmod (struct ovl_data *lo, const char *path, mode_t mode) +do_chmod (struct ovl_data *lo, struct ovl_node *node, const char *path, mode_t mode) { if (lo->xattr_permissions) { - struct ovl_layer *upper = get_upper_layer (lo); struct stat st; - if (upper == NULL) - { - errno = EROFS; - return -1; - } - st.st_uid = 0; st.st_gid = 0; - if (override_mode (upper, -1, path, NULL, &st) < 0 && errno != ENODATA) + if (do_stat (node, -1, path, &st) < 0) return -1; return write_permission_xattr (lo, -1, path, st.st_uid, st.st_gid, mode); @@ -921,14 +924,8 @@ rpl_stat (fuse_req_t req, struct ovl_node *node, int fd, const char *path, struc if (st_in) memcpy (st, st_in, sizeof (*st)); - else if (fd >= 0) - ret = l->ds->fstat (l, fd, path, STATX_BASIC_STATS, st); - else if (path != NULL) - ret = stat (path, st); - else if (node->hidden) - ret = fstatat (node_dirfd (node), node->path, st, AT_SYMLINK_NOFOLLOW); else - ret = l->ds->statat (l, node->path, st, AT_SYMLINK_NOFOLLOW, STATX_BASIC_STATS); + ret = do_stat (node, fd, path, st); if (ret < 0) return ret; @@ -3823,7 +3820,7 @@ ovl_write_buf (fuse_req_t req, fuse_ino_t ino, /* if it is a writepage request, make sure to restore the setuid bit. */ if (fi->writepage && (inode->mode & (S_ISUID | S_ISGID))) { - if (do_fchmod (lo, fi->fh, inode->mode) < 0) + if (do_fchmod (lo, inode->node, fi->fh, inode->mode) < 0) { fuse_reply_err (req, errno); return; @@ -4135,9 +4132,9 @@ ovl_setattr (fuse_req_t req, fuse_ino_t ino, struct stat *attr, int to_set, stru if (to_set & FUSE_SET_ATTR_MODE) { if (fd >= 0) - ret = do_fchmod (lo, fd, attr->st_mode); + ret = do_fchmod (lo, node, fd, attr->st_mode); else - ret = do_chmod (lo, path, attr->st_mode); + ret = do_chmod (lo, node, path, attr->st_mode); if (ret < 0) { fuse_reply_err (req, errno); From 4e2466fe654b98dbfa2142511ec8a4c274d99001 Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Wed, 29 May 2024 16:24:21 +0900 Subject: [PATCH 2/2] main: Do not set -1 for owner overriding xattrs ovl_setattr () used to pass -1 as uid or gid when either of them is not changed for do_fchown () / do_chown (), but if these functions use overriding xattrs instead of real fchown () or chown (), it causes -1 to be written in owner overriding xattrs and break them. Replace -1 with the current uid or gid before calling do_fchown () / do_chown (). Signed-off-by: Akihiko Odaki --- main.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/main.c b/main.c index fc5cc79..5bd1993 100644 --- a/main.c +++ b/main.c @@ -4158,6 +4158,24 @@ ovl_setattr (fuse_req_t req, fuse_ino_t ino, struct stat *attr, int to_set, stru if (uid != -1 || gid != -1) { + struct stat st; + + if (do_stat (node, fd, NULL, &st) < 0) + { + fuse_reply_err (req, errno); + return; + } + + if (uid == -1) + { + uid = st.st_uid; + } + + if (gid == -1) + { + gid = st.st_gid; + } + if (fd >= 0) ret = do_fchown (lo, fd, uid, gid, node->ino->mode); else