[PATCH 0/2] exfat: allow access to paths with trailing dots

[PATCH 0/2] exfat: allow access to paths with trailing dots

[David Disseldorp]

This patchset adds a new exfat "keeptail" mount option, which allows
users to resolve paths carrying trailing period '.' characters.
I'm not a huge fan of "keeptail" as an option name, but couldn't think
of anything better.

Feedback appreciated.

Cheers, David

--

 fs/exfat/exfat_fs.h |  3 ++-
 fs/exfat/namei.c    | 25 ++++++++++++++-----------
 fs/exfat/super.c    |  7 +++++++
 3 files changed, 23 insertions(+), 12 deletions(-)

[PATCH 1/2] exfat: add keeptail mount option

[David Disseldorp]

The "keeptail" mount option will, in a subsequent commit, control
whether or not trailing periods '.' are stripped from path components.

Suggested-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: David Disseldorp <ddiss@suse.de>
---
 fs/exfat/exfat_fs.h | 3 ++-
 fs/exfat/super.c    | 7 +++++++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h
index 1d6da61157c9..af33f867b34e 100644
--- a/fs/exfat/exfat_fs.h
+++ b/fs/exfat/exfat_fs.h
@@ -204,7 +204,8 @@ struct exfat_mount_options {
 	/* on error: continue, panic, remount-ro */
 	enum exfat_error_mode errors;
 	unsigned utf8:1, /* Use of UTF-8 character set */
-		 discard:1; /* Issue discard requests on deletions */
+		 discard:1, /* Issue discard requests on deletions */
+		 keeptail:1; /* Keep trailing periods in paths */
 	int time_offset; /* Offset of timestamps from UTC (in minutes) */
 };
 
diff --git a/fs/exfat/super.c b/fs/exfat/super.c
index 5539ffc20d16..a55c810ffd90 100644
--- a/fs/exfat/super.c
+++ b/fs/exfat/super.c
@@ -173,6 +173,8 @@ static int exfat_show_options(struct seq_file *m, struct dentry *root)
 		seq_puts(m, ",errors=remount-ro");
 	if (opts->discard)
 		seq_puts(m, ",discard");
+	if (opts->keeptail)
+		seq_puts(m, ",keeptail");
 	if (opts->time_offset)
 		seq_printf(m, ",time_offset=%d", opts->time_offset);
 	return 0;
@@ -216,6 +218,7 @@ enum {
 	Opt_charset,
 	Opt_errors,
 	Opt_discard,
+	Opt_keeptail,
 	Opt_time_offset,
 
 	/* Deprecated options */
@@ -242,6 +245,7 @@ static const struct fs_parameter_spec exfat_parameters[] = {
 	fsparam_string("iocharset",		Opt_charset),
 	fsparam_enum("errors",			Opt_errors, exfat_param_enums),
 	fsparam_flag("discard",			Opt_discard),
+	fsparam_flag("keeptail",		Opt_keeptail),
 	fsparam_s32("time_offset",		Opt_time_offset),
 	__fsparam(NULL, "utf8",			Opt_utf8, fs_param_deprecated,
 		  NULL),
@@ -296,6 +300,9 @@ static int exfat_parse_param(struct fs_context *fc, struct fs_parameter *param)
 	case Opt_discard:
 		opts->discard = 1;
 		break;
+	case Opt_keeptail:
+		opts->keeptail = 1;
+		break;
 	case Opt_time_offset:
 		/*
 		 * Make the limit 24 just in case someone invents something
-- 
2.31.1

[PATCH 2/2] exfat: keep trailing dots in paths if keeptail is set

[David Disseldorp]

exfat currently unconditionally strips trailing periods '.' when
performing path lookup. This is done intentionally, loosely following
Windows behaviour and specifications which state:

  #exFAT
  The concatenated file name has the same set of illegal characters as
  other FAT-based file systems (see Table 31).

  #FAT
  ...
  Leading and trailing spaces in a long name are ignored.
  Leading and embedded periods are allowed in a name and are stored in
  the long name. Trailing periods are ignored.

Note: Leading and trailing space ' ' characters are currently retained
by Linux kernel exfat, in conflict with the above specification.

Some implementations, such as fuse-exfat, don't perform path trailer
removal. When mounting images which contain trailing-dot paths, these
paths are unreachable, e.g.:

  + mount.exfat-fuse /dev/zram0 /mnt/test/
  FUSE exfat 1.3.0
  + cd /mnt/test/
  + touch fuse_created_dots... '  fuse_created_spaces  '
  + ls -l
  total 0
  -rwxrwxrwx 1 root 0 0 Aug 18 09:45 '  fuse_created_spaces  '
  -rwxrwxrwx 1 root 0 0 Aug 18 09:45  fuse_created_dots...
  + cd /
  + umount /mnt/test/
  + mount -t exfat /dev/zram0 /mnt/test
  + cd /mnt/test
  + ls -l
  ls: cannot access 'fuse_created_dots...': No such file or directory
  total 0
  -rwxr-xr-x 1 root 0 0 Aug 18 09:45 '  fuse_created_spaces  '
  -????????? ? ?    ? ?            ?  fuse_created_dots...
  + touch kexfat_created_dots... '  kexfat_created_spaces  '
  + ls -l
  ls: cannot access 'fuse_created_dots...': No such file or directory
  total 0
  -rwxr-xr-x 1 root 0 0 Aug 18 09:45 '  fuse_created_spaces  '
  -rwxr-xr-x 1 root 0 0 Aug 18 09:45 '  kexfat_created_spaces  '
  -????????? ? ?    ? ?            ?  fuse_created_dots...
  -rwxr-xr-x 1 root 0 0 Aug 18 09:45  kexfat_created_dots
  + cd /
  + umount /mnt/test/

With this change, the "keeptail" mount option can be used to access
paths with trailing periods. E.g. continuing from the previous example:

  + mount -t exfat -o keeptail /dev/zram0 /mnt/test
  + cd /mnt/test
  + ls -l
  total 0
  -rwxr-xr-x 1 root 0 0 Aug 18 10:32 '  fuse_created_spaces  '
  -rwxr-xr-x 1 root 0 0 Aug 18 10:32 '  kexfat_created_spaces  '
  -rwxr-xr-x 1 root 0 0 Aug 18 10:32  fuse_created_dots...
  -rwxr-xr-x 1 root 0 0 Aug 18 10:32  kexfat_created_dots

Link: https://bugzilla.suse.com/show_bug.cgi?id=1188964
Link: https://lore.kernel.org/linux-fsdevel/003b01d755e4$31fb0d80$95f12880$@samsung.com/
Link: https://docs.microsoft.com/en-us/windows/win32/fileio/exfat-specification#773-filename-field
Suggested-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: David Disseldorp <ddiss@suse.de>
---
 fs/exfat/namei.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c
index 24b41103d1cc..e49455ce6da2 100644
--- a/fs/exfat/namei.c
+++ b/fs/exfat/namei.c
@@ -65,11 +65,14 @@ static int exfat_d_revalidate(struct dentry *dentry, unsigned int flags)
 	return ret;
 }
 
-/* returns the length of a struct qstr, ignoring trailing dots */
-static unsigned int exfat_striptail_len(unsigned int len, const char *name)
+/* returns the length of a struct qstr, possibly ignoring trailing dots */
+static unsigned int exfat_striptail_len(struct super_block *sb,
+					unsigned int len, const char *name)
 {
-	while (len && name[len - 1] == '.')
-		len--;
+	if (!EXFAT_SB(sb)->options.keeptail) {
+		while (len && name[len - 1] == '.')
+			len--;
+	}
 	return len;
 }
 
@@ -83,7 +86,7 @@ static int exfat_d_hash(const struct dentry *dentry, struct qstr *qstr)
 	struct super_block *sb = dentry->d_sb;
 	struct nls_table *t = EXFAT_SB(sb)->nls_io;
 	const unsigned char *name = qstr->name;
-	unsigned int len = exfat_striptail_len(qstr->len, qstr->name);
+	unsigned int len = exfat_striptail_len(sb, qstr->len, qstr->name);
 	unsigned long hash = init_name_hash(dentry);
 	int i, charlen;
 	wchar_t c;
@@ -104,8 +107,8 @@ static int exfat_d_cmp(const struct dentry *dentry, unsigned int len,
 {
 	struct super_block *sb = dentry->d_sb;
 	struct nls_table *t = EXFAT_SB(sb)->nls_io;
-	unsigned int alen = exfat_striptail_len(name->len, name->name);
-	unsigned int blen = exfat_striptail_len(len, str);
+	unsigned int alen = exfat_striptail_len(sb, name->len, name->name);
+	unsigned int blen = exfat_striptail_len(sb, len, str);
 	wchar_t c1, c2;
 	int charlen, i;
 
@@ -136,7 +139,7 @@ static int exfat_utf8_d_hash(const struct dentry *dentry, struct qstr *qstr)
 {
 	struct super_block *sb = dentry->d_sb;
 	const unsigned char *name = qstr->name;
-	unsigned int len = exfat_striptail_len(qstr->len, qstr->name);
+	unsigned int len = exfat_striptail_len(sb, qstr->len, qstr->name);
 	unsigned long hash = init_name_hash(dentry);
 	int i, charlen;
 	unicode_t u;
@@ -161,8 +164,8 @@ static int exfat_utf8_d_cmp(const struct dentry *dentry, unsigned int len,
 		const char *str, const struct qstr *name)
 {
 	struct super_block *sb = dentry->d_sb;
-	unsigned int alen = exfat_striptail_len(name->len, name->name);
-	unsigned int blen = exfat_striptail_len(len, str);
+	unsigned int alen = exfat_striptail_len(sb, name->len, name->name);
+	unsigned int blen = exfat_striptail_len(sb, len, str);
 	unicode_t u_a, u_b;
 	int charlen, i;
 
@@ -419,7 +422,7 @@ static int __exfat_resolve_path(struct inode *inode, const unsigned char *path,
 	struct exfat_inode_info *ei = EXFAT_I(inode);
 
 	/* strip all trailing periods */
-	namelen = exfat_striptail_len(strlen(path), path);
+	namelen = exfat_striptail_len(sb, strlen(path), path);
 	if (!namelen)
 		return -ENOENT;
 
-- 
2.31.1

Re: [PATCH 0/2] exfat: allow access to paths with trailing dots

[Christian Brauner]

On Wed, Aug 18, 2021 at 01:11:21PM +0200, David Disseldorp wrote:
> This patchset adds a new exfat "keeptail" mount option, which allows
> users to resolve paths carrying trailing period '.' characters.
> I'm not a huge fan of "keeptail" as an option name, but couldn't think
> of anything better.

I wouldn't use "period". The vfs uses "dot" and "dotdot" as seen in e.g.
LAST_DOT or LAST_DOTOT. Maybe "keep_last_dot"?

Christian

Re: [PATCH 0/2] exfat: allow access to paths with trailing dots

[David Disseldorp]

On Wed, 18 Aug 2021 14:48:35 +0200, Christian Brauner wrote:

> On Wed, Aug 18, 2021 at 01:11:21PM +0200, David Disseldorp wrote:
> > This patchset adds a new exfat "keeptail" mount option, which allows
> > users to resolve paths carrying trailing period '.' characters.
> > I'm not a huge fan of "keeptail" as an option name, but couldn't think
> > of anything better.  
> 
> I wouldn't use "period". The vfs uses "dot" and "dotdot" as seen in e.g.
> LAST_DOT or LAST_DOTOT. Maybe "keep_last_dot"?

Works for me, although I was under the impression that underscores were
avoided for mount options. Also, I think it'd be clearer as the plural
"keep_last_dots".

Cheers, David

Re: [PATCH 0/2] exfat: allow access to paths with trailing dots

[Christian Brauner]

On Wed, Aug 18, 2021 at 06:40:16PM +0200, David Disseldorp wrote:
> On Wed, 18 Aug 2021 14:48:35 +0200, Christian Brauner wrote:
> 
> > On Wed, Aug 18, 2021 at 01:11:21PM +0200, David Disseldorp wrote:
> > > This patchset adds a new exfat "keeptail" mount option, which allows
> > > users to resolve paths carrying trailing period '.' characters.
> > > I'm not a huge fan of "keeptail" as an option name, but couldn't think
> > > of anything better.  
> > 
> > I wouldn't use "period". The vfs uses "dot" and "dotdot" as seen in e.g.
> > LAST_DOT or LAST_DOTOT. Maybe "keep_last_dot"?
> 
> Works for me, although I was under the impression that underscores were
> avoided for mount options. Also, I think it'd be clearer as the plural

Oh? I' just - for unrelated reasons - looking at cifs code and I
immediately see:

min_enc_offload
max_credits
_netdev

which doesn't necessarily mean that I'm right but rather that that ship
might have sailed. :)

Christian