Skip to contentSkip to conversation

cifs: sanitize multiple delimiters in prepath

PATCH Superseded Thiago Rafael Becker 4 participants 5 messages Dec 15, 2021 – Dec 16, 2021

Subsystems (2)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 cifs: sanitize multiple delimiters in prepath

mount.cifs can pass a device with multiple delimiters in it. This will cause rename(2) to fail with ENOENT.

---
 fs/cifs/fs_context.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)
fs/cifs/fs_context.c +30 -0
index 6a179ae753c1..4ce8a7df3a02 100644
@@ -434,6 +434,34 @@ int smb3_parse_opt(const char *options, const char *key, char **val)
 	return rc;
 }
 
+/*
+ * remove duplicate path delimiters. Windows is supposed to do that
+ * but there are some bugs that prevent rename from working if there are
+ * multiple delimiters.
+ */
+void sanitize_path(char *path) {
+        char *pos = path, last = *path;
+        unsigned int offset = 0;
+
+        while(*(++pos)) {
+                if ((*pos == '/' || *pos == '\\') && (last == '/' || last == '\\')) {
+                        offset++;
+                        continue;
+                }
+
+                last = *pos;
+                *(pos - offset) = *pos;
+        }
+
+        pos = pos - offset - 1;
+
+        /* At this point, there should be only zero or one delimiter at the end of the string */
+        if (*pos != '/' && *pos != '\\')
+                pos++;
+
+        *pos = '\0';
+}
+
 /*
  * Parse a devname into substrings and populate the ctx->UNC and ctx->prepath
  * fields with the result. Returns 0 on success and an error otherwise
@@ -497,6 +525,8 @@ smb3_parse_devname(const char *devname, struct smb3_fs_context *ctx)
 	if (!ctx->prepath)
 		return -ENOMEM;
 
+	sanitize_path(ctx->prepath);
+
 	return 0;
 }
 
Signature · 1 line
2.31.1
#1 lore raw

Steve,

Acked-by: me
Steve, we should have this in stable

Fixes 24e0a1eff9e2 ("cifs: switch to new mount api")
Quoted 61 lines · Thiago Rafael Becker

mount.cifs can pass a device with multiple delimiters in it. This will cause rename(2) to fail with ENOENT.

---
 fs/cifs/fs_context.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)
fs/cifs/fs_context.c +30 -1
index 6a179ae753c1..4ce8a7df3a02 100644
@@ -434,6 +434,34 @@ int smb3_parse_opt(const char *options, const char *key, char **val)
        return rc;
 }
 
+/*
+ * remove duplicate path delimiters. Windows is supposed to do that
+ * but there are some bugs that prevent rename from working if there are
+ * multiple delimiters.
+ */
+void sanitize_path(char *path) {
+        char *pos = path, last = *path;
+        unsigned int offset = 0;
+
+        while(*(++pos)) {
+                if ((*pos == '/' || *pos == '\\') && (last == '/' || last == '\\')) {
+                        offset++;
+                        continue;
+                }
+
+                last = *pos;
+                *(pos - offset) = *pos;
+        }
+
+        pos = pos - offset - 1;
+
+        /* At this point, there should be only zero or one delimiter at the end of the string */
+        if (*pos != '/' && *pos != '\\')
+                pos++;
+
+        *pos = '\0';
+}
+
 /*
  * Parse a devname into substrings and populate the ctx->UNC and ctx->prepath
  * fields with the result. Returns 0 on success and an error otherwise
@@ -497,6 +525,8 @@ smb3_parse_devname(const char *devname, struct smb3_fs_context *ctx)
        if (!ctx->prepath)
                return -ENOMEM;
 
+       sanitize_path(ctx->prepath);
+
        return 0;
 }
 
--

2.31.1

Acked-by Ronnie Sahlberg
#2 lore raw

fixed various whitespace problems (pointed out by checkpatch), added the suggested Acked-by, Fixes etc.
and tentatively merged into cifs-2.6.git for-next pending testing

See attached.

Quoted 73 lines · Leif Sahlberg

Steve,

Acked-by: me
Steve, we should have this in stable

Fixes 24e0a1eff9e2 ("cifs: switch to new mount api")
Quoted 61 lines · Thiago Rafael Becker

mount.cifs can pass a device with multiple delimiters in it. This will cause rename(2) to fail with ENOENT.

---
 fs/cifs/fs_context.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)
fs/cifs/fs_context.c +30 -1
index 6a179ae753c1..4ce8a7df3a02 100644
@@ -434,6 +434,34 @@ int smb3_parse_opt(const char *options, const char *key, char **val)
        return rc;
 }
 
+/*
+ * remove duplicate path delimiters. Windows is supposed to do that
+ * but there are some bugs that prevent rename from working if there are
+ * multiple delimiters.
+ */
+void sanitize_path(char *path) {
+        char *pos = path, last = *path;
+        unsigned int offset = 0;
+
+        while(*(++pos)) {
+                if ((*pos == '/' || *pos == '\\') && (last == '/' || last == '\\')) {
+                        offset++;
+                        continue;
+                }
+
+                last = *pos;
+                *(pos - offset) = *pos;
+        }
+
+        pos = pos - offset - 1;
+
+        /* At this point, there should be only zero or one delimiter at the end of the string */
+        if (*pos != '/' && *pos != '\\')
+                pos++;
+
+        *pos = '\0';
+}
+
 /*
  * Parse a devname into substrings and populate the ctx->UNC and ctx->prepath
  * fields with the result. Returns 0 on success and an error otherwise
@@ -497,6 +525,8 @@ smb3_parse_devname(const char *devname, struct smb3_fs_context *ctx)
        if (!ctx->prepath)
                return -ENOMEM;
 
+       sanitize_path(ctx->prepath);
+
        return 0;
 }
 
--

2.31.1

Signature · 3 lines
Thanks,

Steve

From 859057cc97cc949b0097cd1e776a6708b367e99c Mon Sep 17 00:00:00 2001
From: Thiago Rafael Becker <[email protected]>
Date: Wed, 15 Dec 2021 00:08:31 -0300
Subject: [PATCH] cifs: sanitize multiple delimiters in prepath

mount.cifs can pass a device with multiple delimiters in it. This will cause rename(2) to fail with ENOENT.

---
 fs/cifs/fs_context.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)
fs/cifs/fs_context.c +30 -0
index 6a179ae753c1..4ce8a7df3a02 100644
@@ -434,6 +434,34 @@ int smb3_parse_opt(const char *options, const char *key, char **val)
 	return rc;
 }
 
+/*
+ * remove duplicate path delimiters. Windows is supposed to do that
+ * but there are some bugs that prevent rename from working if there are
+ * multiple delimiters.
+ */
+void sanitize_path(char *path)
+{
+	char *pos = path, last = *path;
+	unsigned int offset = 0;
+
+	while (*(++pos)) {
+		if ((*pos == '/' || *pos == '\\') && (last == '/' || last == '\\')) {
+			offset++;
+			continue;
+		}
+		last = *pos;
+		*(pos - offset) = *pos;
+	}
+
+	pos = pos - offset - 1;
+
+	/* At this point, there should be only zero or one delimiter at the end of the string */
+	if (*pos != '/' && *pos != '\\')
+		pos++;
+
+	*pos = '\0';
+}
+
 /*
  * Parse a devname into substrings and populate the ctx->UNC and ctx->prepath
  * fields with the result. Returns 0 on success and an error otherwise
@@ -497,6 +525,8 @@ smb3_parse_devname(const char *devname, struct smb3_fs_context *ctx)
 	if (!ctx->prepath)
 		return -ENOMEM;
 
+	sanitize_path(ctx->prepath);
+
 	return 0;
 }
 
Signature · 1 line
2.32.0
#3 lore raw

[RFC PATCH] cifs: sanitize_path() can be static

In reply to Thiago Rafael Becker
fs/cifs/fs_context.c:442:6: warning: symbol 'sanitize_path' was not declared. Should it be static?

fs/cifs/fs_context.c:442:6: warning: symbol 'sanitize_path' was not declared. Should it be static?

---
 fs_context.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/cifs/fs_context.c +1 -1
index 4ce8a7df3a02e5..f3557d24664c13 100644
@@ -439,7 +439,7 @@ int smb3_parse_opt(const char *options, const char *key, char **val)
  * but there are some bugs that prevent rename from working if there are
  * multiple delimiters.
  */
-void sanitize_path(char *path) {
+static void sanitize_path(char *path) {
         char *pos = path, last = *path;
         unsigned int offset = 0;
 
#4 lore raw
In reply to Thiago Rafael Becker
Thank you for the patch! Perhaps something to improve:

Hi Thiago,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on cifs/for-next]
[also build test WARNING on v5.16-rc5 next-20211214]
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Thiago-Rafael-Becker/cifs-sanitize-multiple-delimiters-in-prepath/20211215-110931
base:   git://git.samba.org/sfrench/cifs-2.6.git for-next
config: x86_64-randconfig-s022-20211214 (https://download.01.org/0day-ci/archive/20211215/202112151551.AfQ7J0wz-lkp(a)intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.4-dirty
        # https://github.com/0day-ci/linux/commit/e81ed85a4288e898bc0607106d98905be5cd3788
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Thiago-Rafael-Becker/cifs-sanitize-multiple-delimiters-in-prepath/20211215-110931
        git checkout e81ed85a4288e898bc0607106d98905be5cd3788
        # save the config file to linux build tree
        mkdir build_dir
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=x86_64 SHELL=/bin/bash fs/cifs/

If you fix the issue, kindly add following tag as appropriate

All warnings (new ones prefixed by >>):

>> fs/cifs/fs_context.c:442:6: warning: no previous prototype for 'sanitize_path' [-Wmissing-prototypes]
     442 | void sanitize_path(char *path) {
         |      ^~~~~~~~~~~~~

sparse warnings: (new ones prefixed by >>)

>> fs/cifs/fs_context.c:442:6: sparse: sparse: symbol 'sanitize_path' was not declared. Should it be static?

Please review and possibly fold the followup patch.

vim +/sanitize_path +442 fs/cifs/fs_context.c

436
437 /*
438 * remove duplicate path delimiters. Windows is supposed to do that
439 * but there are some bugs that prevent rename from working if there are
440 * multiple delimiters.
441 */

 > 442	void sanitize_path(char *path) {

443 char *pos = path, last = *path;
444 unsigned int offset = 0;
445
446 while(*(++pos)) {
447 if ((*pos == '/' || *pos == '\\') && (last == '/' || last == '\\')) {
448 offset++;
449 continue;
450 }
451
452 last = *pos;
453 *(pos - offset) = *pos;
454 }
455
456 pos = pos - offset - 1;
457
458 /* At this point, there should be only zero or one delimiter at the end of the string */
459 if (*pos != '/' && *pos != '\\')
460 pos++;
461
462 *pos = '\0';
463 }
464

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org