* [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.