* [RFC] libmount: accept X-mount.{owner,group,mode}= @ 2022-03-30 18:13 наб 2022-04-06 11:38 ` Karel Zak 0 siblings, 1 reply; 8+ messages in thread From: наб @ 2022-03-30 18:13 UTC (permalink / raw) To: util-linux [-- Attachment #1: Type: text/plain, Size: 7042 bytes --] Which take an user, group, and mode, respectively, and set them on the target after mounting This is vaguely similar to tmpfs(5)'s [ug]id= and mode= options, but we POSIX-parse the user- and group names Oft requested in systemd/zram-generator, since a common use-case is to use it to create /tmp or an equivalent directory that needs to be a=rwx,o+t (or a user's private temp that needs to be owned by them) ‒ this is impossible without terrible hacks, cf. https://github.com/systemd/zram-generator/issues/150, https://github.com/systemd/zram-generator/issues/146, &c. This started off as a Set{User,Group,Mode}= systemd mount unit, but was poetterung into libmount options: https://github.com/systemd/systemd/pull/22889 --- As a PoC: $ truncate -s40G /tmp/ext4; /sbin/mkfs.ext4 /tmp/ext4 # ./mount -o \ X-mount.owner=nabijaczleweli,X-mount.group=1212,X-mount.mode=1234 \ /tmp/ext4 /tmp/a\ +\ b/ $ l -d /tmp/a\ +\ b/ d-w--wxr-T 3 nabijaczleweli 1212 4.0K Mar 30 20:03 '/tmp/a + b/' I've marked this RFC because the failures are, well, not pretty (for example, given a mis-spelled or unset username): mount: /tmp/a + b: filesystem was mounted, but any subsequent operation failed: Invalid argument. But I'm not sure how to proceed. I've stuffed the parsing and chowning stage into mnt_context_finalize_mount() for ease-of-PoC, but should: (a) the post-syscall error handling in mnt_context_get_mount_excode() be extended to recognise MNT_ERR_MOUNTOPT? (b) the parsing/chowning stages be split (parsing in pre-mount prep, chowning in post-mount)? with a new MNT_ERR_ flag potentially? (c) something else? Best, наб Please keep me in CC, as I'm not subscribed. libmount/src/context_mount.c | 136 +++++++++++++++++++++++++++++++++++ sys-utils/mount.8.adoc | 6 ++ 2 files changed, 142 insertions(+) diff --git a/libmount/src/context_mount.c b/libmount/src/context_mount.c index 1fc3ff2cc..290f55ea7 100644 --- a/libmount/src/context_mount.c +++ b/libmount/src/context_mount.c @@ -1229,6 +1229,138 @@ static int is_source_already_rdonly(struct libmnt_context *cxt) return opts && mnt_optstr_get_option(opts, "ro", NULL, NULL) == 0; } +/* Extracted from mnt_optstr_get_uid() */ +static int parse_ugid(const char *value, size_t valsz, unsigned *uid) +{ + char buf[sizeof(stringify_value(UINT64_MAX))]; + int rc; + uint64_t num; + + assert(value); + assert(uid); + + if (valsz > sizeof(buf) - 1) { + rc = -ERANGE; + goto fail; + } + mem2strcpy(buf, value, valsz, sizeof(buf)); + + rc = ul_strtou64(buf, &num, 10); + if (rc != 0) + goto fail; + if (num > ULONG_MAX || (unsigned) num != num) { + rc = -ERANGE; + goto fail; + } + *uid = (unsigned) num; + + return 0; +fail: + DBG(UTILS, ul_debug("failed to convert '%.*s' to number [rc=%d]", (int) valsz, value, rc)); + return rc; +} + +static int parse_mode(const char *value, size_t valsz, mode_t *uid) +{ + char buf[sizeof(stringify_value(UINT64_MAX))]; + int rc; + uint64_t num; + + assert(value); + assert(uid); + + if (valsz > sizeof(buf) - 1) { + rc = -ERANGE; + goto fail; + } + mem2strcpy(buf, value, valsz, sizeof(buf)); + + rc = ul_strtou64(buf, &num, 8); + if (rc != 0) + goto fail; + if (num > 07777) { + rc = -ERANGE; + goto fail; + } + *uid = (mode_t) num; + + return 0; +fail: + DBG(UTILS, ul_debug("failed to convert '%.*s' to mode [rc=%d]", (int) valsz, value, rc)); + return rc; +} + +/* + * Process X-mount.owner=, X-mount.group=, X-mount.mode=. + */ +static int set_ownership_mode(struct libmnt_context *cxt) +{ + int rc; + + uid_t new_owner = (uid_t) -1; + uid_t new_group = (uid_t) -1; + mode_t new_mode = (mode_t) -1; + + const char *o = mnt_fs_get_user_options(cxt->fs); + + char *owner; + size_t owner_len; + if ((rc = mnt_optstr_get_option(o, "X-mount.owner", &owner, &owner_len)) < 0) + return rc; + if (!rc) { + if (!owner_len) + return -EINVAL; + + char *owner_tofree = NULL; + rc = mnt_get_uid(owner[owner_len] ? (owner_tofree = strndup(owner, owner_len)) : owner, &new_owner); + free(owner_tofree); + if (new_owner == (uid_t) -1 && isdigit(*owner)) + rc = parse_ugid(owner, owner_len, &new_owner); + if (rc) + return rc; + } + + char *group; + size_t group_len; + if ((rc = mnt_optstr_get_option(o, "X-mount.group", &group, &group_len)) < 0) + return rc; + if (!rc) { + if (!group_len) + return -EINVAL; + + char *group_tofree = NULL; + rc = mnt_get_uid(group[group_len] ? (group_tofree = strndup(group, group_len)) : group, &new_group); + free(group_tofree); + if (new_group == (uid_t) -1 && isdigit(*group)) + rc = parse_ugid(group, group_len, &new_group); + if (rc) + return rc; + } + + char *mode; + size_t mode_len; + if ((rc = mnt_optstr_get_option(o, "X-mount.mode", &mode, &mode_len)) < 0) + return rc; + if (!rc) { + if (!group_len) + return -EINVAL; + if ((rc = parse_mode(mode, mode_len, &new_mode))) + return rc; + } + + const char *target = mnt_fs_get_target(cxt->fs); + + if (new_owner != (uid_t) -1 || new_group != (uid_t) -1) + if (lchown(target, new_owner, new_group) == -1) + return -errno; + + if (new_mode != (mode_t) -1) + if (chmod(target, new_mode) == -1) + return -errno; + + return 0; +} + /** * mnt_context_finalize_mount: * @cxt: context @@ -1250,6 +1382,8 @@ int mnt_context_finalize_mount(struct libmnt_context *cxt) rc = mnt_context_prepare_update(cxt); if (!rc) rc = mnt_context_update_tabs(cxt); + if (!rc) + rc = set_ownership_mode(cxt); return rc; } @@ -1328,6 +1462,8 @@ again: rc = mnt_context_do_mount(cxt); if (!rc) rc = mnt_context_update_tabs(cxt); + if (!rc) + rc = set_ownership_mode(cxt); /* * Read-only device or already read-only mounted FS. diff --git a/sys-utils/mount.8.adoc b/sys-utils/mount.8.adoc index 343d7e297..53a0bbb34 100644 --- a/sys-utils/mount.8.adoc +++ b/sys-utils/mount.8.adoc @@ -633,6 +633,12 @@ Allow to make a target directory (mountpoint) if it does not exist yet. The opti **X-mount.subdir=**__directory__:: Allow mounting sub-directory from a filesystem instead of the root directory. For now, this feature is implemented by temporary filesystem root directory mount in unshared namespace and then bind the sub-directory to the final mount point and umount the root of the filesystem. The sub-directory mount shows up atomically for the rest of the system although it is implemented by multiple *mount*(2) syscalls. This feature is EXPERIMENTAL. +*X-mount.owner*=_username_|_UID_, *X-mount.group*=_group_|_GID_:: +Set _mountpoint_'s ownership after mounting. + +*X-mount.mode*=_mode_:: +Set _mountpoint_'s mode after mounting. + *nosymfollow*:: Do not follow symlinks when resolving paths. Symlinks can still be created, and *readlink*(1), *readlink*(2), *realpath*(1), and *realpath*(3) all still work properly. -- 2.30.2 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [RFC] libmount: accept X-mount.{owner,group,mode}= 2022-03-30 18:13 [RFC] libmount: accept X-mount.{owner,group,mode}= наб @ 2022-04-06 11:38 ` Karel Zak 2022-04-07 18:39 ` [RFC v2] " наб 0 siblings, 1 reply; 8+ messages in thread From: Karel Zak @ 2022-04-06 11:38 UTC (permalink / raw) To: наб; +Cc: util-linux On Wed, Mar 30, 2022 at 08:13:00PM +0200, наб wrote: > Which take an user, group, and mode, respectively, and set them on the > target after mounting > > This is vaguely similar to tmpfs(5)'s [ug]id= and mode= options, > but we POSIX-parse the user- and group names > > Oft requested in systemd/zram-generator, since a common use-case > is to use it to create /tmp or an equivalent directory that needs > to be a=rwx,o+t (or a user's private temp that needs to be owned > by them) ‒ this is impossible without terrible hacks, cf. > https://github.com/systemd/zram-generator/issues/150, > https://github.com/systemd/zram-generator/issues/146, &c. > > This started off as a Set{User,Group,Mode}= systemd mount unit, > but was poetterung into libmount options: > https://github.com/systemd/systemd/pull/22889 I see how usable this feature could be, but it also increases complexity of the mount(8) command. > But I'm not sure how to proceed. I've stuffed the parsing and chowning > stage into mnt_context_finalize_mount() for ease-of-PoC, but should: > (a) the post-syscall error handling in mnt_context_get_mount_excode() > be extended to recognise MNT_ERR_MOUNTOPT? Yes, see MNT_ERR_NAMESPACE, it the same thing. We have it in the pre-syscall and in post-syscall sections. > (b) the parsing/chowning stages be split (parsing in pre-mount prep, > chowning in post-mount)? with a new MNT_ERR_ flag potentially? Yes, this is probably the most robust way (don't call mount syscall if X-mount.* is invalid). I guess you can add tgt_owner, tgt_group and tgt_mode to libmnt_context to keep parsed results, fill the fields in mnt_context_prepare_mount() (or so) and use it mnt_context_do_mount() (before mnt_context_switch_ns()). Don't forget to reset it in mnt_reset_context(). Karel -- Karel Zak <kzak@redhat.com> http://karelzak.blogspot.com ^ permalink raw reply [flat|nested] 8+ messages in thread
* [RFC v2] libmount: accept X-mount.{owner,group,mode}= 2022-04-06 11:38 ` Karel Zak @ 2022-04-07 18:39 ` наб 2022-04-13 8:12 ` Karel Zak 2022-04-19 11:13 ` Karel Zak 0 siblings, 2 replies; 8+ messages in thread From: наб @ 2022-04-07 18:39 UTC (permalink / raw) To: Karel Zak; +Cc: util-linux [-- Attachment #1: Type: text/plain, Size: 11258 bytes --] Thanks for the pointers! Please see below for updated scissor-patch, which * splits parsing from setting * integrates parsing into _prepare_mount() * integrates setting into _do_mount() * has tests * returns MNT_ERR_MOUNTOPT (with errno set to an unambiguous value) on parsing errors * returns MNT_ERR_CHOWN/MNT_ERR_CHMOD on setting errors * explicitly mentions that username/group parsing happens in the target namespace, which I assume is what you meant by "before mnt_context_switch_ns()", because that's literal there (I also don't know if that's, uh, valid?); if you meant the transitive namespace switch via mnt_context_switch_target_ns() then it's trivial to hoist it up above it in _prepare() ‒ this is why I marked this RFC Best, наб -- >8 -- Which take an user, group, and mode, respectively, and set them on the target after mounting This is vaguely similar to tmpfs(5)'s [ug]id= and mode= options, but we POSIX-parse the user- and group names Oft requested in systemd/zram-generator, since a common use-case is to use it to create /tmp or an equivalent directory that needs to be a=rwx,o+t (or a user's private temp that needs to be owned by them) ‒ this is impossible without terrible hacks, cf. https://github.com/systemd/zram-generator/issues/150, https://github.com/systemd/zram-generator/issues/146, &c. This started off as a Set{User,Group,Mode}= systemd mount unit, but was poetterung into libmount options: https://github.com/systemd/systemd/pull/22889 --- .gitignore | 1 + libmount/src/context.c | 12 ++- libmount/src/context_mount.c | 157 +++++++++++++++++++++++++++++++++++ libmount/src/libmount.h.in | 13 ++- libmount/src/mountP.h | 4 + sys-utils/mount.8.adoc | 6 ++ 6 files changed, 189 insertions(+), 4 deletions(-) diff --git a/.gitignore b/.gitignore index 840f64615..51a019307 100644 --- a/.gitignore +++ b/.gitignore @@ -71,6 +71,7 @@ ylwrap /blkdiscard /blkzone /blkid +/blkpr /blockdev /cal /cfdisk diff --git a/libmount/src/context.c b/libmount/src/context.c index 99ca58b9f..7927012d2 100644 --- a/libmount/src/context.c +++ b/libmount/src/context.c @@ -57,6 +57,10 @@ struct libmnt_context *mnt_new_context(void) if (!cxt) return NULL; + cxt->tgt_owner = (uid_t) -1; + cxt->tgt_group = (gid_t) -1; + cxt->tgt_mode = (mode_t) -1; + INIT_LIST_HEAD(&cxt->addmounts); ruid = getuid(); @@ -76,7 +80,6 @@ struct libmnt_context *mnt_new_context(void) DBG(CXT, ul_debugobj(cxt, "----> allocate %s", cxt->restricted ? "[RESTRICTED]" : "")); - return cxt; } @@ -130,7 +133,6 @@ void mnt_free_context(struct libmnt_context *cxt) * mnt_context_set_options_pattern(cxt, NULL); * mnt_context_set_target_ns(cxt, NULL); * - * * to reset this stuff. * * Returns: 0 on success, negative number in case of error. @@ -155,6 +157,10 @@ int mnt_reset_context(struct libmnt_context *cxt) free(cxt->orig_user); free(cxt->subdir); + cxt->tgt_owner = (uid_t) -1; + cxt->tgt_group = (gid_t) -1; + cxt->tgt_mode = (mode_t) -1; + cxt->fs = NULL; cxt->mtab = NULL; cxt->utab = NULL; @@ -3108,7 +3114,7 @@ static void close_ns(struct libmnt_ns *ns) * * This function sets errno to ENOSYS and returns error if libmount is * compiled without namespaces support. -* + * * Returns: 0 on success, negative number in case of error. * * Since: 2.33 diff --git a/libmount/src/context_mount.c b/libmount/src/context_mount.c index f09e68860..13209d861 100644 --- a/libmount/src/context_mount.c +++ b/libmount/src/context_mount.c @@ -1020,6 +1020,132 @@ static int do_mount_by_pattern(struct libmnt_context *cxt, const char *pattern) return rc; } +/* Extracted from mnt_optstr_get_uid() */ +static int parse_ugid(const char *value, size_t valsz, unsigned *uid) +{ + char buf[sizeof(stringify_value(UINT64_MAX))]; + int rc; + uint64_t num; + + assert(value); + assert(uid); + + if (valsz > sizeof(buf) - 1) { + rc = -ERANGE; + goto fail; + } + mem2strcpy(buf, value, valsz, sizeof(buf)); + + rc = ul_strtou64(buf, &num, 10); + if (rc != 0) { + errno = ENOENT; + goto fail; + } + if (num > ULONG_MAX || (unsigned) num != num) { + errno = ERANGE; + goto fail; + } + *uid = (unsigned) num; + + return 0; +fail: + DBG(UTILS, ul_debug("failed to convert '%.*s' to number [errno=%d]", (int) valsz, value, errno)); + return -1; +} + +static int parse_mode(const char *value, size_t valsz, mode_t *uid) +{ + char buf[sizeof(stringify_value(UINT64_MAX))]; + int rc; + uint64_t num; + + assert(value); + assert(uid); + + if (valsz > sizeof(buf) - 1) { + rc = -ERANGE; + goto fail; + } + mem2strcpy(buf, value, valsz, sizeof(buf)); + + rc = ul_strtou64(buf, &num, 8); + if (rc != 0) { + errno = EINVAL; + goto fail; + } + if (num > 07777) { + errno = ERANGE; + goto fail; + } + *uid = (mode_t) num; + + return 0; +fail: + DBG(UTILS, ul_debug("failed to convert '%.*s' to mode [errno=%d]", (int) valsz, value, errno)); + return -1; +} + +/* + * Process X-mount.owner=, X-mount.group=, X-mount.mode=. + */ +static int parse_ownership_mode(struct libmnt_context *cxt) +{ + int rc; + + const char *o = mnt_fs_get_user_options(cxt->fs); + if (!o) + return 0; + + char *owner; + size_t owner_len; + if ((rc = mnt_optstr_get_option(o, "X-mount.owner", &owner, &owner_len)) < 0) { + return -MNT_ERR_MOUNTOPT;} + if (!rc) { + if (!owner_len) + return errno = EINVAL, -MNT_ERR_MOUNTOPT; + + char *owner_tofree = NULL; + rc = mnt_get_uid(owner[owner_len] ? (owner_tofree = strndup(owner, owner_len)) : owner, &cxt->tgt_owner); + free(owner_tofree); + if (cxt->tgt_owner == (uid_t) -1 && isdigit(*owner)) + rc = parse_ugid(owner, owner_len, &cxt->tgt_owner); + if (rc) + return -MNT_ERR_MOUNTOPT; + } + + char *group; + size_t group_len; + if ((rc = mnt_optstr_get_option(o, "X-mount.group", &group, &group_len)) < 0) + return -MNT_ERR_MOUNTOPT; + if (!rc) { + if (!group_len) + return errno = EINVAL, -MNT_ERR_MOUNTOPT; + + char *group_tofree = NULL; + rc = mnt_get_uid(group[group_len] ? (group_tofree = strndup(group, group_len)) : group, &cxt->tgt_group); + free(group_tofree); + if (cxt->tgt_group == (uid_t) -1 && isdigit(*group)) + rc = parse_ugid(group, group_len, &cxt->tgt_group); + if (rc) + return errno = ENOENT, -MNT_ERR_MOUNTOPT; + } + + char *mode; + size_t mode_len; + if ((rc = mnt_optstr_get_option(o, "X-mount.mode", &mode, &mode_len)) < 0) + return -MNT_ERR_MOUNTOPT; + if (!rc) { + if (!mode_len) + return errno = EINVAL, -MNT_ERR_MOUNTOPT; + if ((rc = parse_mode(mode, mode_len, &cxt->tgt_mode))) + return -MNT_ERR_MOUNTOPT; + } + + DBG(CXT, ul_debugobj(cxt, "ownership %d:%d, mode %04o", cxt->tgt_owner, cxt->tgt_group, cxt->tgt_mode)); + + return 0; +} + /** * mnt_context_prepare_mount: * @cxt: context @@ -1064,6 +1190,8 @@ int mnt_context_prepare_mount(struct libmnt_context *cxt) rc = mnt_context_guess_fstype(cxt); if (!rc) rc = mnt_context_prepare_target(cxt); + if (!rc) + rc = parse_ownership_mode(cxt); if (!rc) rc = mnt_context_prepare_helper(cxt, "mount", NULL); @@ -1089,6 +1217,21 @@ end: return rc; } +static int set_ownership_mode(struct libmnt_context *cxt) +{ + const char *target = mnt_fs_get_target(cxt->fs); + + if (cxt->tgt_owner != (uid_t) -1 || cxt->tgt_group != (uid_t) -1) + if (lchown(target, cxt->tgt_owner, cxt->tgt_group) == -1) + return -MNT_ERR_CHOWN; + + if (cxt->tgt_mode != (mode_t) -1) + if (chmod(target, cxt->tgt_mode) == -1) + return -MNT_ERR_CHMOD; + + return 0; +} + /** * mnt_context_do_mount * @cxt: context @@ -1191,6 +1334,9 @@ int mnt_context_do_mount(struct libmnt_context *cxt) if (mnt_context_is_veritydev(cxt)) mnt_context_deferred_delete_veritydev(cxt); + if (!res) + res = set_ownership_mode(cxt); + if (!mnt_context_switch_ns(cxt, ns_old)) return -MNT_ERR_NAMESPACE; @@ -1841,7 +1987,18 @@ int mnt_context_get_mount_excode( if (buf) snprintf(buf, bufsz, _("filesystem was mounted, but failed to switch namespace back")); return MNT_EX_SYSERR; + } + if (rc == -MNT_ERR_CHOWN) { + if (buf) + snprintf(buf, bufsz, _("filesystem was mounted, but failed to change ownership to %u:%u: %m"), cxt->tgt_owner, cxt->tgt_group); + return MNT_EX_SYSERR; + } + + if (rc == -MNT_ERR_CHMOD) { + if (buf) + snprintf(buf, bufsz, _("filesystem was mounted, but failed to change mode to %04o: %m"), cxt->tgt_mode); + return MNT_EX_SYSERR; } if (rc < 0) diff --git a/libmount/src/libmount.h.in b/libmount/src/libmount.h.in index 43d8e44ce..0ab1d6ece 100644 --- a/libmount/src/libmount.h.in +++ b/libmount/src/libmount.h.in @@ -220,6 +220,18 @@ enum { * filesystem mounted, but --onlyonce specified */ #define MNT_ERR_ONLYONCE 5010 +/** + * MNT_ERR_CHOWN: + * + * filesystem mounted, but subsequent X-mount.owner=/X-mount.group= lchown(2) failed + */ +#define MNT_ERR_CHOWN 5011 +/** + * MNT_ERR_CHMOD: + * + * filesystem mounted, but subsequent X-mount.mode= chmod(2) failed + */ +#define MNT_ERR_CHMOD 5012 /* @@ -246,7 +258,6 @@ enum { * * [u]mount(8) exit code: out of memory, cannot fork, ... */ - #define MNT_EX_SYSERR 2 /** diff --git a/libmount/src/mountP.h b/libmount/src/mountP.h index 561ddcd8c..c63eb1029 100644 --- a/libmount/src/mountP.h +++ b/libmount/src/mountP.h @@ -294,6 +294,10 @@ struct libmnt_context char *subdir; /* X-mount.subdir= */ + uid_t tgt_owner; /* X-mount.owner= */ + gid_t tgt_group; /* X-mount.group= */ + mode_t tgt_mode; /* X-mount.mode= */ + struct libmnt_fs *fs; /* filesystem description (type, mountpoint, device, ...) */ struct libmnt_fs *fs_template; /* used for @fs on mnt_reset_context() */ diff --git a/sys-utils/mount.8.adoc b/sys-utils/mount.8.adoc index 7465fcd0d..5f0ddd05b 100644 --- a/sys-utils/mount.8.adoc +++ b/sys-utils/mount.8.adoc @@ -636,6 +636,12 @@ Allow to make a target directory (mountpoint) if it does not exist yet. The opti **X-mount.subdir=**__directory__:: Allow mounting sub-directory from a filesystem instead of the root directory. For now, this feature is implemented by temporary filesystem root directory mount in unshared namespace and then bind the sub-directory to the final mount point and umount the root of the filesystem. The sub-directory mount shows up atomically for the rest of the system although it is implemented by multiple *mount*(2) syscalls. This feature is EXPERIMENTAL. +*X-mount.owner*=_username_|_UID_, *X-mount.group*=_group_|_GID_:: +Set _mountpoint_'s ownership after mounting. Names resolved in the target mount namespace, see *-N*. + +*X-mount.mode*=_mode_:: +Set _mountpoint_'s mode after mounting. + *nosymfollow*:: Do not follow symlinks when resolving paths. Symlinks can still be created, and *readlink*(1), *readlink*(2), *realpath*(1), and *realpath*(3) all still work properly. -- 2.30.2 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [RFC v2] libmount: accept X-mount.{owner,group,mode}= 2022-04-07 18:39 ` [RFC v2] " наб @ 2022-04-13 8:12 ` Karel Zak 2022-04-13 13:18 ` наб 2022-04-19 11:13 ` Karel Zak 1 sibling, 1 reply; 8+ messages in thread From: Karel Zak @ 2022-04-13 8:12 UTC (permalink / raw) To: наб; +Cc: util-linux On Thu, Apr 07, 2022 at 08:39:13PM +0200, наб wrote: > Thanks for the pointers! BTW, we have discussion about IDs mapping for filesystems at https://github.com/util-linux/util-linux/issues/1652 The idea is based on user namespaces. Please, see the discussion, maybe it's something what could be attractive for your use-case too. Karel -- Karel Zak <kzak@redhat.com> http://karelzak.blogspot.com ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC v2] libmount: accept X-mount.{owner,group,mode}= 2022-04-13 8:12 ` Karel Zak @ 2022-04-13 13:18 ` наб 0 siblings, 0 replies; 8+ messages in thread From: наб @ 2022-04-13 13:18 UTC (permalink / raw) To: Karel Zak; +Cc: util-linux [-- Attachment #1: Type: text/plain, Size: 765 bytes --] On Wed, Apr 13, 2022 at 10:12:34AM +0200, Karel Zak wrote: > BTW, we have discussion about IDs mapping for filesystems at > https://github.com/util-linux/util-linux/issues/1652 > > The idea is based on user namespaces. Please, see the discussion, > maybe it's something what could be attractive for your use-case too. AFAICT, and as Christian points out, they're entirely unrelated, and idmapping would /not/ be fit for purpose in the zram-generator use-case I'm trying to solve (setting the root to 1777, at the very least; you /could/, I think, finaggle the chown/chmod stuff with idmap but I'd have a hard time recommending it and I'd expect it to be full of footguns a simple chown/chmod avoids, and the ergonomics are better). Best, наб [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC v2] libmount: accept X-mount.{owner,group,mode}= 2022-04-07 18:39 ` [RFC v2] " наб 2022-04-13 8:12 ` Karel Zak @ 2022-04-19 11:13 ` Karel Zak 2022-04-19 14:56 ` [PATCH v3] " наб 1 sibling, 1 reply; 8+ messages in thread From: Karel Zak @ 2022-04-19 11:13 UTC (permalink / raw) To: наб; +Cc: util-linux On Thu, Apr 07, 2022 at 08:39:13PM +0200, наб wrote: > Thanks for the pointers! Some other notes (sorry) ;) > diff --git a/libmount/src/context_mount.c b/libmount/src/context_mount.c > index f09e68860..13209d861 100644 > --- a/libmount/src/context_mount.c > +++ b/libmount/src/context_mount.c > @@ -1020,6 +1020,132 @@ static int do_mount_by_pattern(struct libmnt_context *cxt, const char *pattern) > return rc; > } > > +/* Extracted from mnt_optstr_get_uid() */ > +static int parse_ugid(const char *value, size_t valsz, unsigned *uid) Can you move it to libmount/src/utils.c as mnt_parse_uid() and mnt_parse_gid() ? And use mnt_parse_uid() in libmount/src/optstr.c:mnt_optstr_get_uid()? It would be nice to have only one place (code) where we convert string to uid (or gid). Later we can re-use these functions for ID-mapping patches too. In this case it would be nice to be strict and use uid_t and gid_t in the functions. We already had CVE for this code, so I'd like to be paranoid here :-) > +{ > + char buf[sizeof(stringify_value(UINT64_MAX))]; > + int rc; > + uint64_t num; > + > + assert(value); > + assert(uid); > + > + if (valsz > sizeof(buf) - 1) { > + rc = -ERANGE; > + goto fail; > + } > + mem2strcpy(buf, value, valsz, sizeof(buf)); > + > + rc = ul_strtou64(buf, &num, 10); > + if (rc != 0) { > + errno = ENOENT; > + goto fail; > + } > + if (num > ULONG_MAX || (unsigned) num != num) { || (uid_t) num != num for mnt_parse_uid() and || (gid_t) num != num for mnt_parse_gid() > + errno = ERANGE; > + goto fail; > + } > + *uid = (unsigned) num; > + > + return 0; > +fail: > + DBG(UTILS, ul_debug("failed to convert '%.*s' to number [errno=%d]", (int) valsz, value, errno)); > + return -1; > +} > + > +static int parse_mode(const char *value, size_t valsz, mode_t *uid) Let's also introduce libmount/src/optstr.c:mnt_parse_mode() to be consistent. ... > +/* > + * Process X-mount.owner=, X-mount.group=, X-mount.mode=. > + */ > +static int parse_ownership_mode(struct libmnt_context *cxt) > +{ > + int rc; > + > + const char *o = mnt_fs_get_user_options(cxt->fs); > + if (!o) > + return 0; > + > + char *owner; > + size_t owner_len; Please, please, define variables at the beginning of the code block. > + if ((rc = mnt_optstr_get_option(o, "X-mount.owner", &owner, &owner_len)) < 0) { > + return -MNT_ERR_MOUNTOPT;} > + if (!rc) { > + if (!owner_len) > + return errno = EINVAL, -MNT_ERR_MOUNTOPT; > + > + char *owner_tofree = NULL; here too > + rc = mnt_get_uid(owner[owner_len] ? (owner_tofree = strndup(owner, owner_len)) : owner, &cxt->tgt_owner); > + free(owner_tofree); > + if (cxt->tgt_owner == (uid_t) -1 && isdigit(*owner)) > + rc = parse_ugid(owner, owner_len, &cxt->tgt_owner); > + if (rc) > + return -MNT_ERR_MOUNTOPT; > + } > + > + char *group; > + size_t group_len; here too > + if ((rc = mnt_optstr_get_option(o, "X-mount.group", &group, &group_len)) < 0) > + return -MNT_ERR_MOUNTOPT; > + if (!rc) { > + if (!group_len) > + return errno = EINVAL, -MNT_ERR_MOUNTOPT; > + > + char *group_tofree = NULL; here too > + rc = mnt_get_uid(group[group_len] ? (group_tofree = strndup(group, group_len)) : group, &cxt->tgt_group); > + free(group_tofree); > + if (cxt->tgt_group == (uid_t) -1 && isdigit(*group)) > + rc = parse_ugid(group, group_len, &cxt->tgt_group); > + if (rc) > + return errno = ENOENT, -MNT_ERR_MOUNTOPT; > + } > + > + char *mode; > + size_t mode_len; here too > + if ((rc = mnt_optstr_get_option(o, "X-mount.mode", &mode, &mode_len)) < 0) > + return -MNT_ERR_MOUNTOPT; > + if (!rc) { > + if (!mode_len) > + return errno = EINVAL, -MNT_ERR_MOUNTOPT; > + if ((rc = parse_mode(mode, mode_len, &cxt->tgt_mode))) > + return -MNT_ERR_MOUNTOPT; > + } > + > + DBG(CXT, ul_debugobj(cxt, "ownership %d:%d, mode %04o", cxt->tgt_owner, cxt->tgt_group, cxt->tgt_mode)); > + > + return 0; > +} > + > /** > * mnt_context_prepare_mount: > * @cxt: context > @@ -1064,6 +1190,8 @@ int mnt_context_prepare_mount(struct libmnt_context *cxt) > rc = mnt_context_guess_fstype(cxt); > if (!rc) > rc = mnt_context_prepare_target(cxt); > + if (!rc) > + rc = parse_ownership_mode(cxt); > if (!rc) > rc = mnt_context_prepare_helper(cxt, "mount", NULL); > > @@ -1089,6 +1217,21 @@ end: > return rc; > } > > +static int set_ownership_mode(struct libmnt_context *cxt) > +{ > + const char *target = mnt_fs_get_target(cxt->fs); > + > + if (cxt->tgt_owner != (uid_t) -1 || cxt->tgt_group != (uid_t) -1) Please, add here some debug message, > + if (lchown(target, cxt->tgt_owner, cxt->tgt_group) == -1) > + return -MNT_ERR_CHOWN; > + > + if (cxt->tgt_mode != (mode_t) -1) and here too. > + if (chmod(target, cxt->tgt_mode) == -1) > + return -MNT_ERR_CHMOD; > + > + return 0; > +} > + > /** > * mnt_context_do_mount > * @cxt: context > @@ -1191,6 +1334,9 @@ int mnt_context_do_mount(struct libmnt_context *cxt) > if (mnt_context_is_veritydev(cxt)) > mnt_context_deferred_delete_veritydev(cxt); > > + if (!res) > + res = set_ownership_mode(cxt); > + > if (!mnt_context_switch_ns(cxt, ns_old)) > return -MNT_ERR_NAMESPACE; > > @@ -1841,7 +1987,18 @@ int mnt_context_get_mount_excode( > if (buf) > snprintf(buf, bufsz, _("filesystem was mounted, but failed to switch namespace back")); > return MNT_EX_SYSERR; > + } > > + if (rc == -MNT_ERR_CHOWN) { > + if (buf) > + snprintf(buf, bufsz, _("filesystem was mounted, but failed to change ownership to %u:%u: %m"), cxt->tgt_owner, cxt->tgt_group); > + return MNT_EX_SYSERR; > + } > + > + if (rc == -MNT_ERR_CHMOD) { > + if (buf) > + snprintf(buf, bufsz, _("filesystem was mounted, but failed to change mode to %04o: %m"), cxt->tgt_mode); > + return MNT_EX_SYSERR; > } Yes, this is probably the best solution. Thanks for your work. (Don't forget Signed-off-by:) Karel -- Karel Zak <kzak@redhat.com> http://karelzak.blogspot.com ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3] libmount: accept X-mount.{owner,group,mode}= 2022-04-19 11:13 ` Karel Zak @ 2022-04-19 14:56 ` наб 2022-04-21 10:19 ` Karel Zak 0 siblings, 1 reply; 8+ messages in thread From: наб @ 2022-04-19 14:56 UTC (permalink / raw) To: Karel Zak; +Cc: util-linux [-- Attachment #1: Type: text/plain, Size: 18095 bytes --] Parsing moved to utils.c, optstr.c uses mnt_parse_uid(), all declarations moved to top-of-block, lchown() and chmod() have a debug message before them if they run. Also, re-wrote the test (and actually added them this time :v). On Tue, Apr 19, 2022 at 01:13:41PM +0200, Karel Zak wrote: > In this case it would be nice to be strict and use uid_t and gid_t in > the functions. > > We already had CVE for this code, so I'd like to be paranoid here :-) While in general I agree, and I wanted to do it Properly, util-linux is already very cavalier about this and essentially asserts that [ug]id_t is unsigned (or at least the same size as an unsigned; if that ever changes you're gonna be in CVE city anyway), so I originally gave up trying after not finding any library support for it. Nevertheless, demonomorphised (but the bodies are essentially the same). Updated scissor-patch below. Best, наб -- >8 -- Which take a user, group, and mode, respectively, and set them on the target after mounting This is vaguely similar to tmpfs(5)'s [ug]id= and mode= options, but we POSIX-parse the user- and group names Oft requested in systemd/zram-generator, since a common use-case is to use it to create /tmp or an equivalent directory that needs to be a=rwx,o+t (or a user's private temp that needs to be owned by them) ‒ this is impossible without terrible hacks, cf. https://github.com/systemd/zram-generator/issues/150, https://github.com/systemd/zram-generator/issues/146, &c. This started off as a Set{User,Group,Mode}= systemd mount unit, but was poetterung into libmount options: https://github.com/systemd/systemd/pull/22889 Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz> --- libmount/src/context.c | 12 ++- libmount/src/context_mount.c | 87 +++++++++++++++++++- libmount/src/libmount.h.in | 13 ++- libmount/src/mountP.h | 7 ++ libmount/src/optstr.c | 19 +---- libmount/src/utils.c | 126 +++++++++++++++++++++++++++++ sys-utils/mount.8.adoc | 6 ++ tests/expected/mount/set_ugid_mode | 1 + tests/ts/mount/set_ugid_mode | 65 +++++++++++++++ 9 files changed, 315 insertions(+), 21 deletions(-) create mode 100644 tests/expected/mount/set_ugid_mode create mode 100755 tests/ts/mount/set_ugid_mode diff --git a/libmount/src/context.c b/libmount/src/context.c index 99ca58b9f..7927012d2 100644 --- a/libmount/src/context.c +++ b/libmount/src/context.c @@ -57,6 +57,10 @@ struct libmnt_context *mnt_new_context(void) if (!cxt) return NULL; + cxt->tgt_owner = (uid_t) -1; + cxt->tgt_group = (gid_t) -1; + cxt->tgt_mode = (mode_t) -1; + INIT_LIST_HEAD(&cxt->addmounts); ruid = getuid(); @@ -76,7 +80,6 @@ struct libmnt_context *mnt_new_context(void) DBG(CXT, ul_debugobj(cxt, "----> allocate %s", cxt->restricted ? "[RESTRICTED]" : "")); - return cxt; } @@ -130,7 +133,6 @@ void mnt_free_context(struct libmnt_context *cxt) * mnt_context_set_options_pattern(cxt, NULL); * mnt_context_set_target_ns(cxt, NULL); * - * * to reset this stuff. * * Returns: 0 on success, negative number in case of error. @@ -155,6 +157,10 @@ int mnt_reset_context(struct libmnt_context *cxt) free(cxt->orig_user); free(cxt->subdir); + cxt->tgt_owner = (uid_t) -1; + cxt->tgt_group = (gid_t) -1; + cxt->tgt_mode = (mode_t) -1; + cxt->fs = NULL; cxt->mtab = NULL; cxt->utab = NULL; @@ -3108,7 +3114,7 @@ static void close_ns(struct libmnt_ns *ns) * * This function sets errno to ENOSYS and returns error if libmount is * compiled without namespaces support. -* + * * Returns: 0 on success, negative number in case of error. * * Since: 2.33 diff --git a/libmount/src/context_mount.c b/libmount/src/context_mount.c index f09e68860..6af83e041 100644 --- a/libmount/src/context_mount.c +++ b/libmount/src/context_mount.c @@ -837,7 +837,7 @@ static int do_mount(struct libmnt_context *cxt, const char *try_type) cxt->syscall_status = 0; DBG(CXT, ul_debugobj(cxt, "FAKE mount(2) " - "[source=%s, target=%s, type=%s, " + "[source=%s, target=%s, type=%s," " mountflags=0x%08lx, mountdata=%s]", src, target, type, flags, cxt->mountdata ? "yes" : "<none>")); @@ -862,7 +862,7 @@ static int do_mount(struct libmnt_context *cxt, const char *try_type) } DBG(CXT, ul_debugobj(cxt, "mount(2) " - "[source=%s, target=%s, type=%s, " + "[source=%s, target=%s, type=%s," " mountflags=0x%08lx, mountdata=%s]", src, target, type, flags, cxt->mountdata ? "yes" : "<none>")); @@ -1020,6 +1020,54 @@ static int do_mount_by_pattern(struct libmnt_context *cxt, const char *pattern) return rc; } +/* + * Process X-mount.owner=, X-mount.group=, X-mount.mode=. + */ +static int parse_ownership_mode(struct libmnt_context *cxt) +{ + int rc; + char *value; + size_t valsz; + + const char *o = mnt_fs_get_user_options(cxt->fs); + if (!o) + return 0; + + if ((rc = mnt_optstr_get_option(o, "X-mount.owner", &value, &valsz)) < 0) + return -MNT_ERR_MOUNTOPT; + if (!rc) { + if (!valsz) + return errno = EINVAL, -MNT_ERR_MOUNTOPT; + + if (mnt_parse_uid(value, valsz, &cxt->tgt_owner)) + return -MNT_ERR_MOUNTOPT; + } + + if ((rc = mnt_optstr_get_option(o, "X-mount.group", &value, &valsz)) < 0) + return -MNT_ERR_MOUNTOPT; + if (!rc) { + if (!valsz) + return errno = EINVAL, -MNT_ERR_MOUNTOPT; + + if (mnt_parse_gid(value, valsz, &cxt->tgt_group)) + return -MNT_ERR_MOUNTOPT; + } + + if ((rc = mnt_optstr_get_option(o, "X-mount.mode", &value, &valsz)) < 0) + return -MNT_ERR_MOUNTOPT; + if (!rc) { + if (!valsz) + return errno = EINVAL, -MNT_ERR_MOUNTOPT; + + if ((rc = mnt_parse_mode(value, valsz, &cxt->tgt_mode))) + return -MNT_ERR_MOUNTOPT; + } + + DBG(CXT, ul_debugobj(cxt, "ownership %d:%d, mode %04o", cxt->tgt_owner, cxt->tgt_group, cxt->tgt_mode)); + + return 0; +} + /** * mnt_context_prepare_mount: * @cxt: context @@ -1064,6 +1112,8 @@ int mnt_context_prepare_mount(struct libmnt_context *cxt) rc = mnt_context_guess_fstype(cxt); if (!rc) rc = mnt_context_prepare_target(cxt); + if (!rc) + rc = parse_ownership_mode(cxt); if (!rc) rc = mnt_context_prepare_helper(cxt, "mount", NULL); @@ -1089,6 +1139,25 @@ end: return rc; } +static int set_ownership_mode(struct libmnt_context *cxt) +{ + const char *target = mnt_fs_get_target(cxt->fs); + + if (cxt->tgt_owner != (uid_t) -1 || cxt->tgt_group != (uid_t) -1) { + DBG(CXT, ul_debugobj(cxt, "mount: lchown(%s, %u, %u)", target, cxt->tgt_owner, cxt->tgt_group)); + if (lchown(target, cxt->tgt_owner, cxt->tgt_group) == -1) + return -MNT_ERR_CHOWN; + } + + if (cxt->tgt_mode != (mode_t) -1) { + DBG(CXT, ul_debugobj(cxt, "mount: chmod(%s, %04o)", target, cxt->tgt_mode)); + if (chmod(target, cxt->tgt_mode) == -1) + return -MNT_ERR_CHMOD; + } + + return 0; +} + /** * mnt_context_do_mount * @cxt: context @@ -1191,6 +1260,9 @@ int mnt_context_do_mount(struct libmnt_context *cxt) if (mnt_context_is_veritydev(cxt)) mnt_context_deferred_delete_veritydev(cxt); + if (!res) + res = set_ownership_mode(cxt); + if (!mnt_context_switch_ns(cxt, ns_old)) return -MNT_ERR_NAMESPACE; @@ -1841,7 +1913,18 @@ int mnt_context_get_mount_excode( if (buf) snprintf(buf, bufsz, _("filesystem was mounted, but failed to switch namespace back")); return MNT_EX_SYSERR; + } + + if (rc == -MNT_ERR_CHOWN) { + if (buf) + snprintf(buf, bufsz, _("filesystem was mounted, but failed to change ownership to %u:%u: %m"), cxt->tgt_owner, cxt->tgt_group); + return MNT_EX_SYSERR; + } + if (rc == -MNT_ERR_CHMOD) { + if (buf) + snprintf(buf, bufsz, _("filesystem was mounted, but failed to change mode to %04o: %m"), cxt->tgt_mode); + return MNT_EX_SYSERR; } if (rc < 0) diff --git a/libmount/src/libmount.h.in b/libmount/src/libmount.h.in index 43d8e44ce..0ab1d6ece 100644 --- a/libmount/src/libmount.h.in +++ b/libmount/src/libmount.h.in @@ -220,6 +220,18 @@ enum { * filesystem mounted, but --onlyonce specified */ #define MNT_ERR_ONLYONCE 5010 +/** + * MNT_ERR_CHOWN: + * + * filesystem mounted, but subsequent X-mount.owner=/X-mount.group= lchown(2) failed + */ +#define MNT_ERR_CHOWN 5011 +/** + * MNT_ERR_CHMOD: + * + * filesystem mounted, but subsequent X-mount.mode= chmod(2) failed + */ +#define MNT_ERR_CHMOD 5012 /* @@ -246,7 +258,6 @@ enum { * * [u]mount(8) exit code: out of memory, cannot fork, ... */ - #define MNT_EX_SYSERR 2 /** diff --git a/libmount/src/mountP.h b/libmount/src/mountP.h index 561ddcd8c..26158e8d9 100644 --- a/libmount/src/mountP.h +++ b/libmount/src/mountP.h @@ -109,6 +109,9 @@ extern int mnt_chdir_to_parent(const char *target, char **filename); extern char *mnt_get_username(const uid_t uid); extern int mnt_get_uid(const char *username, uid_t *uid); extern int mnt_get_gid(const char *groupname, gid_t *gid); +extern int mnt_parse_uid(const char *user, size_t user_len, uid_t *gid); +extern int mnt_parse_gid(const char *group, size_t group_len, gid_t *gid); +extern int mnt_parse_mode(const char *mode, size_t mode_len, mode_t *gid); extern int mnt_in_group(gid_t gid); extern int mnt_open_uniq_filename(const char *filename, char **name); @@ -294,6 +297,10 @@ struct libmnt_context char *subdir; /* X-mount.subdir= */ + uid_t tgt_owner; /* X-mount.owner= */ + gid_t tgt_group; /* X-mount.group= */ + mode_t tgt_mode; /* X-mount.mode= */ + struct libmnt_fs *fs; /* filesystem description (type, mountpoint, device, ...) */ struct libmnt_fs *fs_template; /* used for @fs on mnt_reset_context() */ diff --git a/libmount/src/optstr.c b/libmount/src/optstr.c index 43b983ebb..0ad5bfdc6 100644 --- a/libmount/src/optstr.c +++ b/libmount/src/optstr.c @@ -1019,15 +1019,13 @@ int mnt_optstr_fix_user(char **optstr) /* * Converts value from @optstr addressed by @name to uid. * - * Returns: 0 on success, 1 if not found, <0 on error + * Returns: 0 on success, <0 on error */ int mnt_optstr_get_uid(const char *optstr, const char *name, uid_t *uid) { char *value = NULL; size_t valsz = 0; - char buf[sizeof(stringify_value(UINT64_MAX))]; int rc; - uint64_t num; assert(optstr); assert(name); @@ -1037,20 +1035,11 @@ int mnt_optstr_get_uid(const char *optstr, const char *name, uid_t *uid) if (rc != 0) goto fail; - if (valsz > sizeof(buf) - 1) { - rc = -ERANGE; + rc = mnt_parse_uid(value, valsz, uid); + if (rc != 0) { + rc = -errno; goto fail; } - mem2strcpy(buf, value, valsz, sizeof(buf)); - - rc = ul_strtou64(buf, &num, 10); - if (rc != 0) - goto fail; - if (num > ULONG_MAX || (uid_t) num != num) { - rc = -ERANGE; - goto fail; - } - *uid = (uid_t) num; return 0; fail: diff --git a/libmount/src/utils.c b/libmount/src/utils.c index ff3acfb55..a138abe7e 100644 --- a/libmount/src/utils.c +++ b/libmount/src/utils.c @@ -635,6 +635,132 @@ int mnt_get_gid(const char *groupname, gid_t *gid) return rc; } +static int parse_uid_numeric(const char *value, size_t valsz, uid_t *uid) +{ + uint64_t num; + + assert(value); + assert(uid); + + if (valsz > sizeof(stringify_value(UINT64_MAX)) - 1) { + errno = ERANGE; + goto fail; + } + + if (ul_strtou64(value, &num, 10) != 0) { + errno = ENOENT; + goto fail; + } + if (num > ULONG_MAX || (uid_t) num != num) { + errno = ERANGE; + goto fail; + } + *uid = (uid_t) num; + + return 0; +fail: + DBG(UTILS, ul_debug("failed to convert '%s' to number [errno=%d]", value, errno)); + return -1; +} + +/* POSIX-parse user_len-sized user; -1 and errno set, or 0 on success */ +int mnt_parse_uid(const char *user, size_t user_len, uid_t *uid) +{ + char *user_tofree = NULL; + int rc; + + if (user[user_len]) { + user = user_tofree = strndup(user, user_len); + if (!user) + return -1; + } + + rc = mnt_get_uid(user, uid); + if (rc != 0 && isdigit(*user)) + rc = parse_uid_numeric(user, user_len, uid); + + free(user_tofree); + return rc; +} + +static int parse_gid_numeric(const char *value, size_t valsz, gid_t *gid) +{ + uint64_t num; + + assert(value); + assert(gid); + + if (valsz > sizeof(stringify_value(UINT64_MAX)) - 1) { + errno = ERANGE; + goto fail; + } + + if (ul_strtou64(value, &num, 10) != 0) { + errno = ENOENT; + goto fail; + } + if (num > ULONG_MAX || (gid_t) num != num) { + errno = ERANGE; + goto fail; + } + *gid = (gid_t) num; + + return 0; +fail: + DBG(UTILS, ul_debug("failed to convert '%s' to number [errno=%d]", value, errno)); + return -1; +} + +/* POSIX-parse group_len-sized group; -1 and errno set, or 0 on success */ +int mnt_parse_gid(const char *group, size_t group_len, gid_t *gid) +{ + char *group_tofree = NULL; + int rc; + + if (group[group_len]) { + group = group_tofree = strndup(group, group_len); + if (!group) + return -1; + } + + rc = mnt_get_gid(group, gid); + if (rc != 0 && isdigit(*group)) + rc = parse_gid_numeric(group, group_len, gid); + + free(group_tofree); + return rc; +} + +int mnt_parse_mode(const char *mode, size_t mode_len, mode_t *uid) +{ + char buf[sizeof(stringify_value(UINT32_MAX))]; + uint32_t num; + + assert(mode); + assert(uid); + + if (mode_len > sizeof(buf) - 1) { + errno = ERANGE; + goto fail; + } + mem2strcpy(buf, mode, mode_len, sizeof(buf)); + + if (ul_strtou32(buf, &num, 8) != 0) { + errno = EINVAL; + goto fail; + } + if (num > 07777) { + errno = ERANGE; + goto fail; + } + *uid = (mode_t) num; + + return 0; +fail: + DBG(UTILS, ul_debug("failed to convert '%.*s' to mode [errno=%d]", (int) mode_len, mode, errno)); + return -1; +} + int mnt_in_group(gid_t gid) { int rc = 0, n, i; diff --git a/sys-utils/mount.8.adoc b/sys-utils/mount.8.adoc index 7465fcd0d..5f0ddd05b 100644 --- a/sys-utils/mount.8.adoc +++ b/sys-utils/mount.8.adoc @@ -636,6 +636,12 @@ Allow to make a target directory (mountpoint) if it does not exist yet. The opti **X-mount.subdir=**__directory__:: Allow mounting sub-directory from a filesystem instead of the root directory. For now, this feature is implemented by temporary filesystem root directory mount in unshared namespace and then bind the sub-directory to the final mount point and umount the root of the filesystem. The sub-directory mount shows up atomically for the rest of the system although it is implemented by multiple *mount*(2) syscalls. This feature is EXPERIMENTAL. +*X-mount.owner*=_username_|_UID_, *X-mount.group*=_group_|_GID_:: +Set _mountpoint_'s ownership after mounting. Names resolved in the target mount namespace, see *-N*. + +*X-mount.mode*=_mode_:: +Set _mountpoint_'s mode after mounting. + *nosymfollow*:: Do not follow symlinks when resolving paths. Symlinks can still be created, and *readlink*(1), *readlink*(2), *realpath*(1), and *realpath*(3) all still work properly. diff --git a/tests/expected/mount/set_ugid_mode b/tests/expected/mount/set_ugid_mode new file mode 100644 index 000000000..35821117c --- /dev/null +++ b/tests/expected/mount/set_ugid_mode @@ -0,0 +1 @@ +Success diff --git a/tests/ts/mount/set_ugid_mode b/tests/ts/mount/set_ugid_mode new file mode 100755 index 000000000..810f81f38 --- /dev/null +++ b/tests/ts/mount/set_ugid_mode @@ -0,0 +1,65 @@ +#!/bin/bash +# SPDX-License-Identifier: 0BSD + + +TS_TOPDIR="${0%/*}/../.." +TS_DESC="X-mount.{owner,group,mode}=" + +. $TS_TOPDIR/functions.sh +ts_init "$*" + +ts_check_test_command "$TS_CMD_MOUNT" +ts_check_test_command "$TS_CMD_UMOUNT" + +ts_skip_nonroot +ts_check_losetup +ts_check_prog "mkfs.ext2" +ts_check_prog "id" +ts_check_prog "ls" + + +do_one() { + expected="$1"; shift + what="$1"; shift + where="$1"; shift + $TS_CMD_MOUNT "$@" "$what" "$where" >> $TS_OUTPUT 2>> $TS_ERRLOG + read -r m _ o g _ < <(ls -nd "$where") + actual="$m $o $g" + [ "$actual" = "$expected" ] || echo "$*: $actual != $expected" >> $TS_ERRLOG + $TS_CMD_UMOUNT "$where" >> $TS_OUTPUT 2>> $TS_ERRLOG +} + +ts_device_init + +mkfs.ext2 "$TS_LODEV" > /dev/null 2>&1 || ts_die "Cannot make ext2 on $TS_LODEV" +ts_device_has "TYPE" "ext2" "$TS_LODEV" || ts_die "Cannot find ext2 on $TS_LODEV" + +user_1="$(id -un 1)" +group_2="$(id -gn 2)" + + +mkdir -p "$TS_MOUNTPOINT" + +do_one "drwxr-xr-x 0 0" "$TS_LODEV" "$TS_MOUNTPOINT" +do_one "drwxr-xr-x 1 0" "$TS_LODEV" "$TS_MOUNTPOINT" -o "X-mount.owner=$user_1" +do_one "drwxr-xr-x 1 2" "$TS_LODEV" "$TS_MOUNTPOINT" -o "X-mount.group=$group_2" +do_one "d-w--wxr-T 132 2" "$TS_LODEV" "$TS_MOUNTPOINT" -o "X-mount.owner=132,X-mount.mode=1234" +do_one "d-ws-w---x 132 123" "$TS_LODEV" "$TS_MOUNTPOINT" -o "X-mount.mode=4321,X-mount.group=123" +do_one "d-ws-w---x 1 321" "$TS_LODEV" "$TS_MOUNTPOINT" -o "X-mount.owner=$user_1,X-mount.group=321" + + +> "$TS_MOUNTPOINT/bind" +> "$TS_MOUNTPOINT/bindsrc" + +do_one "-rw-r--r-- 0 0" "$TS_MOUNTPOINT/bindsrc" "$TS_MOUNTPOINT/bind" --bind +do_one "-rw-r--r-- 1 0" "$TS_MOUNTPOINT/bindsrc" "$TS_MOUNTPOINT/bind" --bind -o "X-mount.owner=$user_1" +do_one "-rw-r--r-- 1 2" "$TS_MOUNTPOINT/bindsrc" "$TS_MOUNTPOINT/bind" --bind -o "X-mount.group=$group_2" +do_one "--w--wxr-T 132 2" "$TS_MOUNTPOINT/bindsrc" "$TS_MOUNTPOINT/bind" --bind -o "X-mount.owner=132,X-mount.mode=1234" +do_one "--ws-w---x 132 123" "$TS_MOUNTPOINT/bindsrc" "$TS_MOUNTPOINT/bind" --bind -o "X-mount.mode=4321,X-mount.group=123" +do_one "--wx-w---x 1 321" "$TS_MOUNTPOINT/bindsrc" "$TS_MOUNTPOINT/bind" --bind -o "X-mount.owner=$user_1,X-mount.group=321" + + +rm -fd "$TS_MOUNTPOINT/bind" "$TS_MOUNTPOINT/bindsrc" "$TS_MOUNTPOINT" + +ts_log "Success" +ts_finalize -- 2.30.2 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3] libmount: accept X-mount.{owner,group,mode}= 2022-04-19 14:56 ` [PATCH v3] " наб @ 2022-04-21 10:19 ` Karel Zak 0 siblings, 0 replies; 8+ messages in thread From: Karel Zak @ 2022-04-21 10:19 UTC (permalink / raw) To: наб; +Cc: util-linux On Tue, Apr 19, 2022 at 04:56:07PM +0200, наб wrote: > libmount/src/context.c | 12 ++- > libmount/src/context_mount.c | 87 +++++++++++++++++++- > libmount/src/libmount.h.in | 13 ++- > libmount/src/mountP.h | 7 ++ > libmount/src/optstr.c | 19 +---- > libmount/src/utils.c | 126 +++++++++++++++++++++++++++++ > sys-utils/mount.8.adoc | 6 ++ > tests/expected/mount/set_ugid_mode | 1 + > tests/ts/mount/set_ugid_mode | 65 +++++++++++++++ > 9 files changed, 315 insertions(+), 21 deletions(-) > create mode 100644 tests/expected/mount/set_ugid_mode > create mode 100755 tests/ts/mount/set_ugid_mode Applied. Thanks for all the work and patience. I think the result is pretty usable and readable :-) I did some additional small changes to the code: https://github.com/util-linux/util-linux/commit/24e896c1400c2328b8bdffde674a3d74429acdf1 Karel -- Karel Zak <kzak@redhat.com> http://karelzak.blogspot.com ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-04-21 10:20 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-03-30 18:13 [RFC] libmount: accept X-mount.{owner,group,mode}= наб 2022-04-06 11:38 ` Karel Zak 2022-04-07 18:39 ` [RFC v2] " наб 2022-04-13 8:12 ` Karel Zak 2022-04-13 13:18 ` наб 2022-04-19 11:13 ` Karel Zak 2022-04-19 14:56 ` [PATCH v3] " наб 2022-04-21 10:19 ` Karel Zak
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.