| Message ID | 20241112101006.30715-1-mszeredi@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | statmount: add flag to retrieve unescaped options | expand |
On Tue, 2024-11-12 at 11:10 +0100, Miklos Szeredi wrote: > Filesystem options can be retrieved with STATMOUNT_MNT_OPTS, which > returns a string of comma separated options, where some characters are > escaped using the \OOO notation. > > Add a new flag, STATMOUNT_OPT_ARRAY, which instead returns the raw > option values separated with '\0' charaters. > > Since escaped charaters are rare, this inteface is preferable for > non-libmount users which likley don't want to deal with option > de-escaping. > > Example code: > > if (st->mask & STATMOUNT_OPT_ARRAY) { > const char *opt = st->str + st->opt_array; > > for (unsigned int i = 0; i < st->opt_num; i++) { > printf("opt_array[%i]: <%s>\n", i, opt); > opt += strlen(opt) + 1; > } > } > If the options are separated by NULs, how does userland know where to stop? At some point we will probably end up adding a new string value that would go after the opt array, and userland will need some way to clearly tell where that new string begins and the NUL-terminated options array ends. > Signed-off-by: Miklos Szeredi <mszeredi@redhat.com> > --- > fs/namespace.c | 42 ++++++++++++++++++++++++++++++++++++++ > include/uapi/linux/mount.h | 7 +++++-- > 2 files changed, 47 insertions(+), 2 deletions(-) > > diff --git a/fs/namespace.c b/fs/namespace.c > index 9a4ab1bc8b94..a16f75011610 100644 > --- a/fs/namespace.c > +++ b/fs/namespace.c > @@ -5074,6 +5074,41 @@ static int statmount_mnt_opts(struct kstatmount *s, struct seq_file *seq) > return 0; > } > > +static int statmount_opt_array(struct kstatmount *s, struct seq_file *seq) > +{ > + struct vfsmount *mnt = s->mnt; > + struct super_block *sb = mnt->mnt_sb; > + size_t start = seq->count; > + u32 count = 0; > + char *p, *end, *next, *u = seq->buf + start; > + int err; > + > + if (!sb->s_op->show_options) > + return 0; > + > + err = sb->s_op->show_options(seq, mnt->mnt_root); > + if (err) > + return err; > + > + if (unlikely(seq_has_overflowed(seq))) > + return -EAGAIN; > + > + end = seq->buf + seq->count; > + *end = '\0'; > + for (p = u + 1; p < end; p = next + 1) { > + next = strchrnul(p, ','); > + *next = '\0'; > + u += string_unescape(p, u, 0, UNESCAPE_OCTAL) + 1; > + count++; > + if (!count) > + return -EOVERFLOW; > + } > + seq->count = u - 1 - seq->buf; > + s->sm.opt_num = count; > + > + return 0; > +} > + > static int statmount_string(struct kstatmount *s, u64 flag) > { > int ret = 0; > @@ -5099,6 +5134,10 @@ static int statmount_string(struct kstatmount *s, u64 flag) > sm->mnt_opts = start; > ret = statmount_mnt_opts(s, seq); > break; > + case STATMOUNT_OPT_ARRAY: > + sm->opt_array = start; > + ret = statmount_opt_array(s, seq); > + break; > case STATMOUNT_FS_SUBTYPE: > sm->fs_subtype = start; > statmount_fs_subtype(s, seq); > @@ -5252,6 +5291,9 @@ static int do_statmount(struct kstatmount *s, u64 mnt_id, u64 mnt_ns_id, > if (!err && s->mask & STATMOUNT_MNT_OPTS) > err = statmount_string(s, STATMOUNT_MNT_OPTS); > > + if (!err && s->mask & STATMOUNT_OPT_ARRAY) > + err = statmount_string(s, STATMOUNT_OPT_ARRAY); > + > if (!err && s->mask & STATMOUNT_FS_SUBTYPE) > err = statmount_string(s, STATMOUNT_FS_SUBTYPE); > > diff --git a/include/uapi/linux/mount.h b/include/uapi/linux/mount.h > index 2b49e9131d77..c0fda4604187 100644 > --- a/include/uapi/linux/mount.h > +++ b/include/uapi/linux/mount.h > @@ -154,7 +154,7 @@ struct mount_attr { > */ > struct statmount { > __u32 size; /* Total size, including strings */ > - __u32 mnt_opts; /* [str] Mount options of the mount */ > + __u32 mnt_opts; /* [str] Options (comma separated, escaped) */ > __u64 mask; /* What results were written */ > __u32 sb_dev_major; /* Device ID */ > __u32 sb_dev_minor; > @@ -175,7 +175,9 @@ struct statmount { > __u64 mnt_ns_id; /* ID of the mount namespace */ > __u32 fs_subtype; /* [str] Subtype of fs_type (if any) */ > __u32 sb_source; /* [str] Source string of the mount */ > - __u64 __spare2[48]; > + __u32 opt_num; /* Number of fs options */ > + __u32 opt_array; /* [str] Array of nul terminated fs options */ > + __u64 __spare2[47]; > char str[]; /* Variable size part containing strings */ > }; > > @@ -211,6 +213,7 @@ struct mnt_id_req { > #define STATMOUNT_MNT_OPTS 0x00000080U /* Want/got mnt_opts */ > #define STATMOUNT_FS_SUBTYPE 0x00000100U /* Want/got fs_subtype */ > #define STATMOUNT_SB_SOURCE 0x00000200U /* Want/got sb_source */ > +#define STATMOUNT_OPT_ARRAY 0x00000400U /* Want/got opt_... */ > > /* > * Special @mnt_id values that can be passed to listmount
On Tue, 12 Nov 2024 at 13:20, Jeff Layton <jlayton@kernel.org> wrote: > If the options are separated by NULs, how does userland know where to > stop? The number of options is returned in st->opt_num. I find that clearer than returning the end offset. Thanks, Miklos
On Tue, 2024-11-12 at 07:20 -0500, Jeff Layton wrote: > On Tue, 2024-11-12 at 11:10 +0100, Miklos Szeredi wrote: > > Filesystem options can be retrieved with STATMOUNT_MNT_OPTS, which > > returns a string of comma separated options, where some characters are > > escaped using the \OOO notation. > > > > Add a new flag, STATMOUNT_OPT_ARRAY, which instead returns the raw > > option values separated with '\0' charaters. > > > > Since escaped charaters are rare, this inteface is preferable for > > non-libmount users which likley don't want to deal with option > > de-escaping. > > > > Example code: > > > > if (st->mask & STATMOUNT_OPT_ARRAY) { > > const char *opt = st->str + st->opt_array; > > > > for (unsigned int i = 0; i < st->opt_num; i++) { > > printf("opt_array[%i]: <%s>\n", i, opt); > > opt += strlen(opt) + 1; > > } > > } > > > > If the options are separated by NULs, how does userland know where to > stop? > > At some point we will probably end up adding a new string value that > would go after the opt array, and userland will need some way to > clearly tell where that new string begins and the NUL-terminated > options array ends. > Ok, that should work. > > Signed-off-by: Miklos Szeredi <mszeredi@redhat.com> > > --- > > fs/namespace.c | 42 ++++++++++++++++++++++++++++++++++++++ > > include/uapi/linux/mount.h | 7 +++++-- > > 2 files changed, 47 insertions(+), 2 deletions(-) > > > > diff --git a/fs/namespace.c b/fs/namespace.c > > index 9a4ab1bc8b94..a16f75011610 100644 > > --- a/fs/namespace.c > > +++ b/fs/namespace.c > > @@ -5074,6 +5074,41 @@ static int statmount_mnt_opts(struct kstatmount *s, struct seq_file *seq) > > return 0; > > } > > > > +static int statmount_opt_array(struct kstatmount *s, struct seq_file *seq) > > +{ > > + struct vfsmount *mnt = s->mnt; > > + struct super_block *sb = mnt->mnt_sb; > > + size_t start = seq->count; > > + u32 count = 0; > > + char *p, *end, *next, *u = seq->buf + start; > > + int err; > > + > > + if (!sb->s_op->show_options) > > + return 0; > > + > > + err = sb->s_op->show_options(seq, mnt->mnt_root); > > + if (err) > > + return err; > > + > > + if (unlikely(seq_has_overflowed(seq))) > > + return -EAGAIN; > > + > > + end = seq->buf + seq->count; > > + *end = '\0'; > > + for (p = u + 1; p < end; p = next + 1) { > > + next = strchrnul(p, ','); > > + *next = '\0'; > > + u += string_unescape(p, u, 0, UNESCAPE_OCTAL) + 1; > > + count++; > > + if (!count) > > + return -EOVERFLOW; > > + } > > + seq->count = u - 1 - seq->buf; > > + s->sm.opt_num = count; > > + > > + return 0; > > +} > > + > > static int statmount_string(struct kstatmount *s, u64 flag) > > { > > int ret = 0; > > @@ -5099,6 +5134,10 @@ static int statmount_string(struct kstatmount *s, u64 flag) > > sm->mnt_opts = start; > > ret = statmount_mnt_opts(s, seq); > > break; > > + case STATMOUNT_OPT_ARRAY: > > + sm->opt_array = start; > > + ret = statmount_opt_array(s, seq); > > + break; > > case STATMOUNT_FS_SUBTYPE: > > sm->fs_subtype = start; > > statmount_fs_subtype(s, seq); > > @@ -5252,6 +5291,9 @@ static int do_statmount(struct kstatmount *s, u64 mnt_id, u64 mnt_ns_id, > > if (!err && s->mask & STATMOUNT_MNT_OPTS) > > err = statmount_string(s, STATMOUNT_MNT_OPTS); > > > > + if (!err && s->mask & STATMOUNT_OPT_ARRAY) > > + err = statmount_string(s, STATMOUNT_OPT_ARRAY); > > + > > if (!err && s->mask & STATMOUNT_FS_SUBTYPE) > > err = statmount_string(s, STATMOUNT_FS_SUBTYPE); > > > > diff --git a/include/uapi/linux/mount.h b/include/uapi/linux/mount.h > > index 2b49e9131d77..c0fda4604187 100644 > > --- a/include/uapi/linux/mount.h > > +++ b/include/uapi/linux/mount.h > > @@ -154,7 +154,7 @@ struct mount_attr { > > */ > > struct statmount { > > __u32 size; /* Total size, including strings */ > > - __u32 mnt_opts; /* [str] Mount options of the mount */ > > + __u32 mnt_opts; /* [str] Options (comma separated, escaped) */ > > __u64 mask; /* What results were written */ > > __u32 sb_dev_major; /* Device ID */ > > __u32 sb_dev_minor; > > @@ -175,7 +175,9 @@ struct statmount { > > __u64 mnt_ns_id; /* ID of the mount namespace */ > > __u32 fs_subtype; /* [str] Subtype of fs_type (if any) */ > > __u32 sb_source; /* [str] Source string of the mount */ > > - __u64 __spare2[48]; > > + __u32 opt_num; /* Number of fs options */ Since there are 2 ways to get mount options, maybe make this more clear: "Number of fs options in opt_array". > > + __u32 opt_array; /* [str] Array of nul terminated fs options */ > > + __u64 __spare2[47]; > > char str[]; /* Variable size part containing strings */ > > }; > > > > @@ -211,6 +213,7 @@ struct mnt_id_req { > > #define STATMOUNT_MNT_OPTS 0x00000080U /* Want/got mnt_opts */ > > #define STATMOUNT_FS_SUBTYPE 0x00000100U /* Want/got fs_subtype */ > > #define STATMOUNT_SB_SOURCE 0x00000200U /* Want/got sb_source */ > > +#define STATMOUNT_OPT_ARRAY 0x00000400U /* Want/got opt_... */ > > > > /* > > * Special @mnt_id values that can be passed to listmount > Acked-by: Jeff Layton <jlayton@kernel.org>
On Tue, Nov 12, 2024 at 11:10:04AM +0100, Miklos Szeredi wrote: > Filesystem options can be retrieved with STATMOUNT_MNT_OPTS, which > returns a string of comma separated options, where some characters are > escaped using the \OOO notation. > > Add a new flag, STATMOUNT_OPT_ARRAY, which instead returns the raw > option values separated with '\0' charaters. > > Since escaped charaters are rare, this inteface is preferable for > non-libmount users which likley don't want to deal with option > de-escaping. > > Example code: > > if (st->mask & STATMOUNT_OPT_ARRAY) { > const char *opt = st->str + st->opt_array; > > for (unsigned int i = 0; i < st->opt_num; i++) { > printf("opt_array[%i]: <%s>\n", i, opt); > opt += strlen(opt) + 1; > } > } > > Signed-off-by: Miklos Szeredi <mszeredi@redhat.com> > --- I'm likely going to snatch this for v6.13 still. I just need to reflow the code as the formatting is broken and maybe rename a few variables and need to wrap my head around the parsing. I'm testing this now.
On Wed, Nov 13, 2024 at 01:27:08PM +0100, Christian Brauner wrote: > On Tue, Nov 12, 2024 at 11:10:04AM +0100, Miklos Szeredi wrote: > > Filesystem options can be retrieved with STATMOUNT_MNT_OPTS, which > > returns a string of comma separated options, where some characters are > > escaped using the \OOO notation. > > > > Add a new flag, STATMOUNT_OPT_ARRAY, which instead returns the raw > > option values separated with '\0' charaters. > > > > Since escaped charaters are rare, this inteface is preferable for > > non-libmount users which likley don't want to deal with option > > de-escaping. > > > > Example code: > > > > if (st->mask & STATMOUNT_OPT_ARRAY) { > > const char *opt = st->str + st->opt_array; > > > > for (unsigned int i = 0; i < st->opt_num; i++) { > > printf("opt_array[%i]: <%s>\n", i, opt); > > opt += strlen(opt) + 1; > > } > > } > > > > Signed-off-by: Miklos Szeredi <mszeredi@redhat.com> > > --- > > I'm likely going to snatch this for v6.13 still. I just need to reflow > the code as the formatting is broken and maybe rename a few variables > and need to wrap my head around the parsing. I'm testing this now. Please take a look at the top of #vfs.misc and tell me whether this is ok with you.
On Wed, 13 Nov 2024 at 17:31, Christian Brauner <brauner@kernel.org> wrote: > Please take a look at the top of #vfs.misc and tell me whether this is > ok with you. One more thing I'd do is: - if (seq->count == start) + if (seq->count <= start + 1) This would handle the (broken) case of just a single comma in the options. So it's not a bug fix per-se, but would make it clear that the loop will run at least once, and so the seq->count calculation at the end won't be skewed. The buf_start calculation could also be moved further down before the loop, where it's actually used. I don't find the variable naming much better, how about taking the naming from string_unescape: opt_start -> src - pointer to escaped string buf_start -> dst - pointer to de-escaped string The others make sense. Thanks, Miklos
diff --git a/fs/namespace.c b/fs/namespace.c index 9a4ab1bc8b94..a16f75011610 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -5074,6 +5074,41 @@ static int statmount_mnt_opts(struct kstatmount *s, struct seq_file *seq) return 0; } +static int statmount_opt_array(struct kstatmount *s, struct seq_file *seq) +{ + struct vfsmount *mnt = s->mnt; + struct super_block *sb = mnt->mnt_sb; + size_t start = seq->count; + u32 count = 0; + char *p, *end, *next, *u = seq->buf + start; + int err; + + if (!sb->s_op->show_options) + return 0; + + err = sb->s_op->show_options(seq, mnt->mnt_root); + if (err) + return err; + + if (unlikely(seq_has_overflowed(seq))) + return -EAGAIN; + + end = seq->buf + seq->count; + *end = '\0'; + for (p = u + 1; p < end; p = next + 1) { + next = strchrnul(p, ','); + *next = '\0'; + u += string_unescape(p, u, 0, UNESCAPE_OCTAL) + 1; + count++; + if (!count) + return -EOVERFLOW; + } + seq->count = u - 1 - seq->buf; + s->sm.opt_num = count; + + return 0; +} + static int statmount_string(struct kstatmount *s, u64 flag) { int ret = 0; @@ -5099,6 +5134,10 @@ static int statmount_string(struct kstatmount *s, u64 flag) sm->mnt_opts = start; ret = statmount_mnt_opts(s, seq); break; + case STATMOUNT_OPT_ARRAY: + sm->opt_array = start; + ret = statmount_opt_array(s, seq); + break; case STATMOUNT_FS_SUBTYPE: sm->fs_subtype = start; statmount_fs_subtype(s, seq); @@ -5252,6 +5291,9 @@ static int do_statmount(struct kstatmount *s, u64 mnt_id, u64 mnt_ns_id, if (!err && s->mask & STATMOUNT_MNT_OPTS) err = statmount_string(s, STATMOUNT_MNT_OPTS); + if (!err && s->mask & STATMOUNT_OPT_ARRAY) + err = statmount_string(s, STATMOUNT_OPT_ARRAY); + if (!err && s->mask & STATMOUNT_FS_SUBTYPE) err = statmount_string(s, STATMOUNT_FS_SUBTYPE); diff --git a/include/uapi/linux/mount.h b/include/uapi/linux/mount.h index 2b49e9131d77..c0fda4604187 100644 --- a/include/uapi/linux/mount.h +++ b/include/uapi/linux/mount.h @@ -154,7 +154,7 @@ struct mount_attr { */ struct statmount { __u32 size; /* Total size, including strings */ - __u32 mnt_opts; /* [str] Mount options of the mount */ + __u32 mnt_opts; /* [str] Options (comma separated, escaped) */ __u64 mask; /* What results were written */ __u32 sb_dev_major; /* Device ID */ __u32 sb_dev_minor; @@ -175,7 +175,9 @@ struct statmount { __u64 mnt_ns_id; /* ID of the mount namespace */ __u32 fs_subtype; /* [str] Subtype of fs_type (if any) */ __u32 sb_source; /* [str] Source string of the mount */ - __u64 __spare2[48]; + __u32 opt_num; /* Number of fs options */ + __u32 opt_array; /* [str] Array of nul terminated fs options */ + __u64 __spare2[47]; char str[]; /* Variable size part containing strings */ }; @@ -211,6 +213,7 @@ struct mnt_id_req { #define STATMOUNT_MNT_OPTS 0x00000080U /* Want/got mnt_opts */ #define STATMOUNT_FS_SUBTYPE 0x00000100U /* Want/got fs_subtype */ #define STATMOUNT_SB_SOURCE 0x00000200U /* Want/got sb_source */ +#define STATMOUNT_OPT_ARRAY 0x00000400U /* Want/got opt_... */ /* * Special @mnt_id values that can be passed to listmount
Filesystem options can be retrieved with STATMOUNT_MNT_OPTS, which returns a string of comma separated options, where some characters are escaped using the \OOO notation. Add a new flag, STATMOUNT_OPT_ARRAY, which instead returns the raw option values separated with '\0' charaters. Since escaped charaters are rare, this inteface is preferable for non-libmount users which likley don't want to deal with option de-escaping. Example code: if (st->mask & STATMOUNT_OPT_ARRAY) { const char *opt = st->str + st->opt_array; for (unsigned int i = 0; i < st->opt_num; i++) { printf("opt_array[%i]: <%s>\n", i, opt); opt += strlen(opt) + 1; } } Signed-off-by: Miklos Szeredi <mszeredi@redhat.com> --- fs/namespace.c | 42 ++++++++++++++++++++++++++++++++++++++ include/uapi/linux/mount.h | 7 +++++-- 2 files changed, 47 insertions(+), 2 deletions(-)