[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

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

[Namjae Jeon]

> Date: Wed, 18 Aug 2021 13:11:21 +0200
> From: David Disseldorp <ddiss@suse.de>
> To: linux-fsdevel@vger.kernel.org
> Subject: [PATCH 0/2] exfat: allow access to paths with trailing dots
> 
> 
Hi David,
> 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.
We are concerning that this mount option allow to create the filename that
contain trailing period. It will cause the compatibility issues with windows.
I think compatibility with windows is more important than fuse-exfat.

Can we only allow those files to be accessed and prevented from being created?
> 
> Feedback appreciated.

Thanks!
> 
> 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(-)