diff mbox series

[04/25] vfs: Implement parameter value retrieval with fsinfo() [ver #13]

Message ID 155905629702.1662.7233272785972036117.stgit@warthog.procyon.org.uk (mailing list archive)
State New, archived
Headers show
Series VFS: Introduce filesystem information query syscall [ver #13] | expand

Commit Message

David Howells May 28, 2019, 3:11 p.m. UTC
Implement parameter value retrieval with fsinfo() - akin to parsing
/proc/mounts.

This allows all the parameters to be retrieved in one go with:

	struct fsinfo_params params = {
		.request	= FSINFO_ATTR_PARAMETER,
	};

Each parameter comes as a pair of blobs with a length tacked on the front
rather than using separators, since any printable character that could be
used as a separator can be found in some value somewhere (including comma).
In fact, cifs allows the separator to be set using the "sep=" option in
parameter parsing.

The length on the front of each blob is 1-3 bytes long.  Each byte has a
flag in bit 7 that's set if there are more bytes and clear on the last
byte; bits 0-6 should be shifted and OR'd into the length count.  The bytes
are most-significant first.

For example, 0x83 0xf5 0x06 is the length (0x03<<14 | 0x75<<7 | 0x06).

As mentioned, each parameter comes as a pair of blobs in key, value order.
The value has length zero if not present.  So, for example:

	\x08compress\x04zlib

from btrfs would be equivalent to "compress=zlib" and:

	\x02ro\x00\x06noexec\x00

would be equivalent to "ro,noexec".

The test-fsinfo sample program is modified to dump the parameters.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/fsinfo.c                 |  114 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/fsinfo.h      |    3 +
 include/uapi/linux/fsinfo.h |    1 
 samples/vfs/test-fsinfo.c   |   38 ++++++++++++++
 4 files changed, 156 insertions(+)

Comments

Miklos Szeredi May 29, 2019, 8:08 a.m. UTC | #1
On Tue, May 28, 2019 at 5:11 PM David Howells <dhowells@redhat.com> wrote:
>
> Implement parameter value retrieval with fsinfo() - akin to parsing
> /proc/mounts.
>
> This allows all the parameters to be retrieved in one go with:
>
>         struct fsinfo_params params = {
>                 .request        = FSINFO_ATTR_PARAMETER,
>         };

Ah, here it is.

>
> Each parameter comes as a pair of blobs with a length tacked on the front
> rather than using separators, since any printable character that could be
> used as a separator can be found in some value somewhere (including comma).
> In fact, cifs allows the separator to be set using the "sep=" option in
> parameter parsing.
>
> The length on the front of each blob is 1-3 bytes long.  Each byte has a
> flag in bit 7 that's set if there are more bytes and clear on the last
> byte; bits 0-6 should be shifted and OR'd into the length count.  The bytes
> are most-significant first.
>
> For example, 0x83 0xf5 0x06 is the length (0x03<<14 | 0x75<<7 | 0x06).

Sounds way too complicated.  What about fixed 4byte sizes?  Or using
the nul charater as separator (and binary blobs be damned)?

[...]

> +static void fsinfo_insert_sb_flag_parameters(struct path *path,
> +                                            struct fsinfo_kparams *params)
> +{
> +       int s_flags = READ_ONCE(path->dentry->d_sb->s_flags);
> +
> +       if (s_flags & SB_DIRSYNC)
> +               fsinfo_note_param(params, "dirsync", NULL);
> +       if (s_flags & SB_LAZYTIME)
> +               fsinfo_note_param(params, "lazytime", NULL);
> +       if (s_flags & SB_MANDLOCK)
> +               fsinfo_note_param(params, "mand", NULL);
> +       if (s_flags & SB_POSIXACL)
> +               fsinfo_note_param(params, "posixacl", NULL);
> +       if (s_flags & SB_RDONLY)
> +               fsinfo_note_param(params, "ro", NULL);
> +       if (s_flags & SB_SYNCHRONOUS)
> +               fsinfo_note_param(params, "sync", NULL);

Again, don't blindly transform s_flags into options, because some of
them may have been internally manipulated by the filesystem.

You could do a helper for filesystems that does the the common ones
(ro/sync/dirsync) but all of that *should* go through the filesystem.

Same goes for vfs_parse_sb_flag() btw.   It should be moved into each
filesystem's ->parse_param() and not be a mandatory thing.

Thanks,
Miklos
David Howells June 18, 2019, 10:34 p.m. UTC | #2
Miklos Szeredi <miklos@szeredi.hu> wrote:

> Again, don't blindly transform s_flags into options, because some of
> them may have been internally manipulated by the filesystem.

In what filesystems do I need to undo this manipulation?

> You could do a helper for filesystems that does the the common ones
> (ro/sync/dirsync) but all of that *should* go through the filesystem.

I don't agree, but since you keep insisting, I've changed the helper function
that renders these so that it now takes s_flags as an argument and is called
from generic_fsinfo() if the filesystem doesn't handle FSINFO_ATTR_PARAMETERS.

Therefore, every filesystem that handles FSINFO_ATTR_PARAMETERS, *must* call
the function itself (or do the noting directly) otherwise these parameters
will not get rendered.

The helper function has been exported, and the calling filesystem can give any
s_flags it likes.  All the filesystems so far just use
path->dentry->d_sb->s_flags.

> Same goes for vfs_parse_sb_flag() btw.   It should be moved into each
> filesystem's ->parse_param() and not be a mandatory thing.

I disagree.  Every filesystem *must* be able to accept these standard flags,
even if it then ignores them.

David
Miklos Szeredi June 19, 2019, 6:33 a.m. UTC | #3
On Wed, Jun 19, 2019 at 12:34 AM David Howells <dhowells@redhat.com> wrote:

> > Same goes for vfs_parse_sb_flag() btw.   It should be moved into each
> > filesystem's ->parse_param() and not be a mandatory thing.
>
> I disagree.  Every filesystem *must* be able to accept these standard flags,
> even if it then ignores them.

"posixacl" is not a standard flag.  It never was accepted by mount(8)
so I don't see where you got that from.

Can you explain why you think "mand", "sync", "dirsync", "lazytime"
should be accepted by a filesystem such as proc?  The argument that it
breaks userspace is BS, because this is a new interface, hence by
definition we cannot break old userspace.  If mount(8) wants to use
the new API and there really is breakage if these options are rejected
(which I doubt) then it can easily work around that by ignoring them
itself.

Also why should "rw" not be rejected for filesystems which are
read-only by definition, such as iso9660?

Thanks,
Miklos
diff mbox series

Patch

diff --git a/fs/fsinfo.c b/fs/fsinfo.c
index c7f9c894c737..2da321b34bdf 100644
--- a/fs/fsinfo.c
+++ b/fs/fsinfo.c
@@ -283,6 +283,25 @@  static int fsinfo_generic_param_enum(struct file_system_type *f,
 	return sizeof(*p);
 }
 
+static void fsinfo_insert_sb_flag_parameters(struct path *path,
+					     struct fsinfo_kparams *params)
+{
+	int s_flags = READ_ONCE(path->dentry->d_sb->s_flags);
+
+	if (s_flags & SB_DIRSYNC)
+		fsinfo_note_param(params, "dirsync", NULL);
+	if (s_flags & SB_LAZYTIME)
+		fsinfo_note_param(params, "lazytime", NULL);
+	if (s_flags & SB_MANDLOCK)
+		fsinfo_note_param(params, "mand", NULL);
+	if (s_flags & SB_POSIXACL)
+		fsinfo_note_param(params, "posixacl", NULL);
+	if (s_flags & SB_RDONLY)
+		fsinfo_note_param(params, "ro", NULL);
+	if (s_flags & SB_SYNCHRONOUS)
+		fsinfo_note_param(params, "sync", NULL);
+}
+
 /*
  * Implement some queries generically from stuff in the superblock.
  */
@@ -345,8 +364,17 @@  static int vfs_fsinfo(struct path *path, struct fsinfo_kparams *params)
 		return fsinfo(path, params);
 
 	while (!signal_pending(current)) {
+		if (params->request == FSINFO_ATTR_PARAMETERS) {
+			if (down_read_killable(&dentry->d_sb->s_umount) < 0)
+				return -ERESTARTSYS;
+			fsinfo_insert_sb_flag_parameters(path, params);
+		}
+
 		params->usage = 0;
 		ret = fsinfo(path, params);
+		if (params->request == FSINFO_ATTR_PARAMETERS)
+			up_read(&dentry->d_sb->s_umount);
+
 		if (ret <= (int)params->buf_size)
 			return ret; /* Error or it fitted */
 		kvfree(params->buffer);
@@ -504,6 +532,7 @@  static const struct fsinfo_attr_info fsinfo_buffer_info[FSINFO_ATTR__NR] = {
 	FSINFO_STRUCT		(PARAM_DESCRIPTION,	param_description),
 	FSINFO_STRUCT_N		(PARAM_SPECIFICATION,	param_specification),
 	FSINFO_STRUCT_N		(PARAM_ENUM,		param_enum),
+	FSINFO_OPAQUE		(PARAMETERS,		-),
 };
 
 /**
@@ -644,3 +673,88 @@  SYSCALL_DEFINE5(fsinfo,
 error:
 	return ret;
 }
+
+/*
+ * Store a parameter into the user's parameter buffer.  The key is prefixed by
+ * a single byte length (1-127) and the value by one (0-0x7f) or two bytes
+ * (0x80-0x3fff) or three bytes (0x4000-0x1fffff).
+ *
+ * Note that we must always make the size determination, even if the buffer is
+ * already full, so that we can tell the caller how much buffer we actually
+ * need.
+ */
+static void __fsinfo_note_param(struct fsinfo_kparams *params, const char *key,
+				const char *val, unsigned int vlen)
+{
+	char *p;
+	unsigned int usage;
+	int klen, total, vmeta;
+	u8 x;
+
+	klen = strlen(key);
+	BUG_ON(klen < 1 || klen > 127);
+	BUG_ON(vlen > (1 << 21) - 1);
+	BUG_ON(vlen > 0 && !val);
+
+	vmeta = (vlen <= 127) ? 1 : (vlen <= 127 * 127) ? 2 : 3;
+
+	total = 1 + klen + vmeta + vlen;
+
+	usage = params->usage;
+	params->usage = usage + total;
+	if (!params->buffer || params->usage > params->buf_size)
+		return;
+
+	p = params->buffer + usage;
+	*p++ = klen;
+	p = memcpy(p, key, klen);
+	p += klen;
+
+	/* The more significant groups of 7 bits in the size are included in
+	 * most->least order with 0x80 OR'd in.  The least significant 7 bits
+	 * are last with the top bit clear.
+	 */
+	x = vlen >> 14;
+	if (x & 0x7f)
+		*p++ = 0x80 | x;
+
+	x = vlen >> 7;
+	if (x & 0x7f)
+		*p++ = 0x80 | x;
+
+	*p++ = vlen & 0x7f;
+	memcpy(p, val, vlen);
+}
+
+/**
+ * fsinfo_note_param - Store a parameter for FSINFO_ATTR_PARAMETERS
+ * @params: The parameter buffer
+ * @key: The parameter's key
+ * @val: The parameter's value (or NULL)
+ */
+void fsinfo_note_param(struct fsinfo_kparams *params, const char *key,
+		       const char *val)
+{
+	__fsinfo_note_param(params, key, val, val ? strlen(val) : 0);
+}
+EXPORT_SYMBOL(fsinfo_note_param);
+
+/**
+ * fsinfo_note_paramf - Store a formatted parameter for FSINFO_ATTR_PARAMETERS
+ * @params: The parameter buffer
+ * @key: The parameter's key
+ * @val_fmt: Format string for the parameter's value
+ */
+void fsinfo_note_paramf(struct fsinfo_kparams *params, const char *key,
+			const char *val_fmt, ...)
+{
+	va_list va;
+	int n;
+
+	va_start(va, val_fmt);
+	n = vsnprintf(params->scratch_buffer, 4096, val_fmt, va);
+	va_end(va);
+
+	__fsinfo_note_param(params, key, params->scratch_buffer, n);
+}
+EXPORT_SYMBOL(fsinfo_note_paramf);
diff --git a/include/linux/fsinfo.h b/include/linux/fsinfo.h
index e17e4f0bae18..3383027a6e9d 100644
--- a/include/linux/fsinfo.h
+++ b/include/linux/fsinfo.h
@@ -29,6 +29,9 @@  struct fsinfo_kparams {
 };
 
 extern int generic_fsinfo(struct path *, struct fsinfo_kparams *);
+extern void fsinfo_note_param(struct fsinfo_kparams *, const char *, const char *);
+extern void fsinfo_note_paramf(struct fsinfo_kparams *, const char *, const char *, ...)
+	__printf(3, 4);
 
 static inline void fsinfo_set_cap(struct fsinfo_capabilities *c,
 				  enum fsinfo_capability cap)
diff --git a/include/uapi/linux/fsinfo.h b/include/uapi/linux/fsinfo.h
index 248c3c4a1e32..0f134847e88b 100644
--- a/include/uapi/linux/fsinfo.h
+++ b/include/uapi/linux/fsinfo.h
@@ -30,6 +30,7 @@  enum fsinfo_attribute {
 	FSINFO_ATTR_PARAM_DESCRIPTION	= 12,	/* General fs parameter description */
 	FSINFO_ATTR_PARAM_SPECIFICATION	= 13,	/* Nth parameter specification */
 	FSINFO_ATTR_PARAM_ENUM		= 14,	/* Nth enum-to-val */
+	FSINFO_ATTR_PARAMETERS		= 15,	/* Mount parameters (large string) */
 	FSINFO_ATTR__NR
 };
 
diff --git a/samples/vfs/test-fsinfo.c b/samples/vfs/test-fsinfo.c
index 71c7b68e76b3..2960fa2b9843 100644
--- a/samples/vfs/test-fsinfo.c
+++ b/samples/vfs/test-fsinfo.c
@@ -81,6 +81,7 @@  static const struct fsinfo_attr_info fsinfo_buffer_info[FSINFO_ATTR__NR] = {
 	FSINFO_STRUCT		(PARAM_DESCRIPTION,	param_description),
 	FSINFO_STRUCT_N		(PARAM_SPECIFICATION,	param_specification),
 	FSINFO_STRUCT_N		(PARAM_ENUM,		param_enum),
+	FSINFO_OVERLARGE	(PARAMETERS,		-),
 };
 
 #define FSINFO_NAME(X,Y) [FSINFO_ATTR_##X] = #Y
@@ -100,6 +101,7 @@  static const char *fsinfo_attr_names[FSINFO_ATTR__NR] = {
 	FSINFO_NAME		(PARAM_DESCRIPTION,	param_description),
 	FSINFO_NAME		(PARAM_SPECIFICATION,	param_specification),
 	FSINFO_NAME		(PARAM_ENUM,		param_enum),
+	FSINFO_NAME		(PARAMETERS,		parameters),
 };
 
 union reply {
@@ -345,6 +347,34 @@  static void dump_fsinfo(enum fsinfo_attribute attr,
 	dumper(r, size);
 }
 
+static void dump_params(struct fsinfo_attr_info about, union reply *r, int size)
+{
+	int len;
+	char *p = r->buffer, *e = p + size;
+	bool is_key = true;
+
+	while (p < e) {
+		len = 0;
+		while (p[0] & 0x80) {
+			len <<= 7;
+			len |= *p++ & 0x7f;
+		}
+
+		len <<= 7;
+		len |= *p++;
+		if (len > e - p)
+			break;
+		if (is_key || len)
+			printf("%s%*.*s", is_key ? "[PARM] " : "= ", len, len, p);
+		if (is_key)
+			putchar(' ');
+		else
+			putchar('\n');
+		p += len;
+		is_key = !is_key;
+	}
+}
+
 /*
  * Try one subinstance of an attribute.
  */
@@ -420,6 +450,12 @@  static int try_one(const char *file, struct fsinfo_params *params, bool raw)
 		return 0;
 	}
 
+	switch (params->request) {
+	case FSINFO_ATTR_PARAMETERS:
+		if (ret == 0)
+			return 0;
+	}
+
 	switch (about.flags & (__FSINFO_N | __FSINFO_NM)) {
 	case 0:
 		printf("\e[33m%s\e[m: ",
@@ -462,6 +498,8 @@  static int try_one(const char *file, struct fsinfo_params *params, bool raw)
 		return 0;
 
 	case __FSINFO_OVER:
+		if (params->request == FSINFO_ATTR_PARAMETERS)
+			dump_params(about, r, ret);
 		return 0;
 
 	case __FSINFO_STRUCT_ARRAY: