[RFC] does # really need to be escaped in devnames?

David Howells dhowells at redhat.com
Fri Sep 12 11:45:49 PDT 2025


How about the attached?  I've changed the way the volume is displayed in
/proc/mounts to use the numeric volume ID, e.g.:

	openafs.org:536870922~root.doc on /afs/openafs.org/doc type afs (ro,relatime,flock=local)

but included the volume name as kind of a comment as it's easier for human eye
to comprehend.  I've also altered the afs mount devname parser so that it can
handle this sort of devname and to discard the "comment".

We don't actually need to display the [#%] preference marker or the
.readonly/.backup suffixes as they're only really relevant when resolving a
mountpoint.  See:

	https://docs.openafs.org/AdminGuide/HDRWQ208.html#HDRWQ209

(we only need to know what sort of mount we're on and if we're given an
explicit instruction).

David
---
afs: Fix devname used in /proc/mounts

There's an issue with the way kafs mounts are displayed in /proc/mounts and
supplied in the mount device name: the devname may begin with either a '#'
or a '%'.  The former is a problem, however, as some tools interpret this
as a comment (e.g. getmntent(3)).

There is another pair of issues as well in that the volume name may be
changed whilst the volume is mounted and the volume name may be remapped to
a different volume.  Really, the volume name should only be considered
during mount and the volume ID (which is part of the primary key to sget())
should be used instead where possible.  That said, it's more user-friendly
to see the volume name in /proc/mounts.

Fix this by:

 (1) Change the kafs device name used in /proc/mounts such that it shows
     the cell and the volume number with the current volume name after a
     tilda, e.g.:

        openafs.org:536870922~root.doc on /afs/grand.central.org/doc type afs (ro,relatime,flock=local)

 (2) Alter the mount name parsing so that it accepts:
        "[<cell>:]<volid>[~comment]"

     as the device name, thereby allowing stuff scraped from /proc/mounts
     to be fed back, e.g.:

        mount -t afs openafs.org:536870922~root.doc /mnt
        mount -t afs openafs.org:536870922 /mnt
        mount -t afs 536870922 /mnt

     The "comment" is ignored, allowing the current volume name (which is
     subject to third party changes) to also be displayed.

The parsing of the AFS mountpoint target format is retained both for
backward compatibility and to be able to handle automounting, e.g.:

        mount -t afs \#openafs.org:root.doc /mnt
        ls /afs/openafs.org/doc

Note that this may surprise any tools that expect to see the old forms of
kafs line in /proc/mounts.

Reported-by: Siddhesh Poyarekar <siddhesh at gotplt.org>
Signed-off-by: David Howells <dhowells at redhat.com>
cc: Marc Dionne <marc.dionne at auristor.com>
cc: Jeffrey Altman <jaltman at auristor.com>
cc: Al Viro <viro at zeniv.linux.org.uk>
cc: Linus Torvalds <torvalds at linux-foundation.org>
cc: Jan Kara <jack at suse.cz>
cc: Ian Kent <raven at themaw.net>
cc: Christian Brauner <brauner at kernel.org>
cc: linux-afs at lists.infradead.org
cc: linux-fsdevel at vger.kernel.org
---
 fs/afs/internal.h |    1 
 fs/afs/super.c    |  105 +++++++++++++++++++++++++++++++-----------------------
 2 files changed, 63 insertions(+), 43 deletions(-)

diff --git a/fs/afs/internal.h b/fs/afs/internal.h
index 1124ea4000cb..392c1df4651a 100644
--- a/fs/afs/internal.h
+++ b/fs/afs/internal.h
@@ -53,6 +53,7 @@ struct afs_fs_context {
 	bool			autocell;	/* T if set auto mount operation */
 	bool			dyn_root;	/* T if dynamic root */
 	bool			no_cell;	/* T if the source is "none" (for dynroot) */
+	bool			by_vid;		/* T if mount by volume ID */
 	enum afs_flock_mode	flock_mode;	/* Partial file-locking emulation mode */
 	afs_voltype_t		type;		/* type of volume requested */
 	unsigned int		volnamesz;	/* size of volume name */
diff --git a/fs/afs/super.c b/fs/afs/super.c
index da407f2d6f0d..9b1d8ac39261 100644
--- a/fs/afs/super.c
+++ b/fs/afs/super.c
@@ -158,29 +158,13 @@ static int afs_show_devname(struct seq_file *m, struct dentry *root)
 	struct afs_super_info *as = AFS_FS_S(root->d_sb);
 	struct afs_volume *volume = as->volume;
 	struct afs_cell *cell = as->cell;
-	const char *suf = "";
-	char pref = '%';
 
 	if (as->dyn_root) {
 		seq_puts(m, "none");
 		return 0;
 	}
 
-	switch (volume->type) {
-	case AFSVL_RWVOL:
-		break;
-	case AFSVL_ROVOL:
-		pref = '#';
-		if (volume->type_force)
-			suf = ".readonly";
-		break;
-	case AFSVL_BACKVOL:
-		pref = '#';
-		suf = ".backup";
-		break;
-	}
-
-	seq_printf(m, "%c%s:%s%s", pref, cell->name, volume->name, suf);
+	seq_printf(m, "%s:%llu~%s", cell->name, volume->vid, volume->name);
 	return 0;
 }
 
@@ -219,12 +203,16 @@ static int afs_show_options(struct seq_file *m, struct dentry *root)
  *	"#[cell:]volume.readonly"	R/O volume
  *	"%[cell:]volume.backup"		Backup volume
  *	"#[cell:]volume.backup"		Backup volume
+ *
+ * or:
+ *	[cell:]volumeid[~ignored-vol-name]
  */
 static int afs_parse_source(struct fs_context *fc, struct fs_parameter *param)
 {
 	struct afs_fs_context *ctx = fc->fs_private;
 	struct afs_cell *cell;
 	const char *cellname, *suffix, *name = param->string;
+	char qual = 0;
 	int cellnamesz;
 
 	_enter(",%s", name);
@@ -232,33 +220,39 @@ static int afs_parse_source(struct fs_context *fc, struct fs_parameter *param)
 	if (fc->source)
 		return invalf(fc, "kAFS: Multiple sources not supported");
 
-	if (!name) {
+	if (!name || !name[0]) {
 		printk(KERN_ERR "kAFS: no volume name specified\n");
 		return -EINVAL;
 	}
 
-	if ((name[0] != '%' && name[0] != '#') || !name[1]) {
-		/* To use dynroot, we don't want to have to provide a source */
-		if (strcmp(name, "none") == 0) {
-			ctx->no_cell = true;
-			return 0;
-		}
-		printk(KERN_ERR "kAFS: unparsable volume name\n");
-		return -EINVAL;
+	/* To use dynroot, we don't want to have to provide a source */
+	if (strcmp(name, "none") == 0) {
+		ctx->no_cell = true;
+		return 0;
 	}
 
 	/* determine the type of volume we're looking for */
-	if (name[0] == '%') {
+	switch (name[0]) {
+	case '%':
 		ctx->type = AFSVL_RWVOL;
 		ctx->force = true;
+		fallthrough;
+	case '#':
+		qual = *name++;
+		if (!*name)
+			goto unparseable;
+		break;
 	}
-	name++;
 
 	/* split the cell name out if there is one */
 	ctx->volname = strchr(name, ':');
 	if (ctx->volname) {
 		cellname = name;
 		cellnamesz = ctx->volname - name;
+		if (!cellnamesz) {
+			printk(KERN_ERR "kAFS: Empty cell name specified\n");
+			return -EINVAL;
+		}
 		ctx->volname++;
 	} else {
 		ctx->volname = name;
@@ -266,24 +260,45 @@ static int afs_parse_source(struct fs_context *fc, struct fs_parameter *param)
 		cellnamesz = 0;
 	}
 
-	/* the volume type is further affected by a possible suffix */
-	suffix = strrchr(ctx->volname, '.');
-	if (suffix) {
-		if (strcmp(suffix, ".readonly") == 0) {
-			ctx->type = AFSVL_ROVOL;
-			ctx->force = true;
-		} else if (strcmp(suffix, ".backup") == 0) {
-			ctx->type = AFSVL_BACKVOL;
-			ctx->force = true;
-		} else if (suffix[1] == 0) {
-		} else {
-			suffix = NULL;
+	if (!qual) {
+		const char *cp = ctx->volname;
+
+		while (isdigit(*cp))
+			cp++;
+		if (cp == ctx->volname) {
+			printk(KERN_ERR "kAFS: Empty volume ID specified\n");
+			return -EINVAL;
+		}
+		if (*cp != 0 && *cp != '~') {
+			printk(KERN_ERR "kAFS: Expected decimal volume ID\n");
+			return -EINVAL;
 		}
-	}
 
-	ctx->volnamesz = suffix ?
-		suffix - ctx->volname : strlen(ctx->volname);
+		ctx->by_vid	= true;
+		ctx->volnamesz	= cp - ctx->volname;
+	} else {
+		/* the volume type is further affected by a possible suffix */
+		suffix = strrchr(ctx->volname, '.');
+		if (suffix) {
+			if (strcmp(suffix, ".readonly") == 0) {
+				ctx->type = AFSVL_ROVOL;
+				ctx->force = true;
+			} else if (strcmp(suffix, ".backup") == 0) {
+				ctx->type = AFSVL_BACKVOL;
+				ctx->force = true;
+			} else if (suffix[1] == 0) {
+			} else {
+				suffix = NULL;
+			}
+		}
 
+		ctx->volnamesz = suffix ?
+			suffix - ctx->volname : strlen(ctx->volname);
+		if (!ctx->volnamesz) {
+			printk(KERN_ERR "kAFS: Empty volume name specified\n");
+			return -EINVAL;
+		}
+	}
 	_debug("cell %*.*s [%p]",
 	       cellnamesz, cellnamesz, cellname ?: "", ctx->cell);
 
@@ -310,6 +325,10 @@ static int afs_parse_source(struct fs_context *fc, struct fs_parameter *param)
 	fc->source = param->string;
 	param->string = NULL;
 	return 0;
+
+unparseable:
+	printk(KERN_ERR "kAFS: unparsable volume name\n");
+	return -EINVAL;
 }
 
 /*




More information about the linux-afs mailing list