All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/1] cifs: sanitize multiple delimiters in prepath
@ 2021-12-15  3:08 Thiago Rafael Becker
  2021-12-15  6:59 ` Leif Sahlberg
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Thiago Rafael Becker @ 2021-12-15  3:08 UTC (permalink / raw)
  To: linux-cifs; +Cc: Steve French, linux-kernel, lsahlber, Thiago Rafael Becker

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

BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=2031200
Signed-off-by: Thiago Rafael Becker <trbecker@gmail.com>
---
 fs/cifs/fs_context.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/fs/cifs/fs_context.c b/fs/cifs/fs_context.c
index 6a179ae753c1..4ce8a7df3a02 100644
--- a/fs/cifs/fs_context.c
+++ b/fs/cifs/fs_context.c
@@ -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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/1] cifs: sanitize multiple delimiters in prepath
  2021-12-15  3:08 [PATCH 1/1] cifs: sanitize multiple delimiters in prepath Thiago Rafael Becker
@ 2021-12-15  6:59 ` Leif Sahlberg
  2021-12-16  3:14   ` Steve French
  2021-12-15  7:55 ` [RFC PATCH] cifs: sanitize_path() can be static kernel test robot
  2021-12-15  8:02 ` [PATCH 1/1] cifs: sanitize multiple delimiters in prepath kernel test robot
  2 siblings, 1 reply; 5+ messages in thread
From: Leif Sahlberg @ 2021-12-15  6:59 UTC (permalink / raw)
  To: Thiago Rafael Becker; +Cc: CIFS, Steve French, linux-kernel

Steve,

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

    Fixes: 24e0a1eff9e2 ("cifs: switch to new mount api")
    Cc: stable@vger.kernel.org # 5.11+


On Wed, Dec 15, 2021 at 1:09 PM Thiago Rafael Becker <trbecker@gmail.com> wrote:
>
> mount.cifs can pass a device with multiple delimiters in it. This will
> cause rename(2) to fail with ENOENT.
>
> BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=2031200
> Signed-off-by: Thiago Rafael Becker <trbecker@gmail.com>
> ---
>  fs/cifs/fs_context.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>
> diff --git a/fs/cifs/fs_context.c b/fs/cifs/fs_context.c
> index 6a179ae753c1..4ce8a7df3a02 100644
> --- a/fs/cifs/fs_context.c
> +++ b/fs/cifs/fs_context.c
> @@ -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
>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [RFC PATCH] cifs: sanitize_path() can be static
  2021-12-15  3:08 [PATCH 1/1] cifs: sanitize multiple delimiters in prepath Thiago Rafael Becker
  2021-12-15  6:59 ` Leif Sahlberg
@ 2021-12-15  7:55 ` kernel test robot
  2021-12-15  8:02 ` [PATCH 1/1] cifs: sanitize multiple delimiters in prepath kernel test robot
  2 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2021-12-15  7:55 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 791 bytes --]

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

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: kernel test robot <lkp@intel.com>
---
 fs_context.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/cifs/fs_context.c b/fs/cifs/fs_context.c
index 4ce8a7df3a02e5..f3557d24664c13 100644
--- a/fs/cifs/fs_context.c
+++ b/fs/cifs/fs_context.c
@@ -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;
 

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/1] cifs: sanitize multiple delimiters in prepath
  2021-12-15  3:08 [PATCH 1/1] cifs: sanitize multiple delimiters in prepath Thiago Rafael Becker
  2021-12-15  6:59 ` Leif Sahlberg
  2021-12-15  7:55 ` [RFC PATCH] cifs: sanitize_path() can be static kernel test robot
@ 2021-12-15  8:02 ` kernel test robot
  2 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2021-12-15  8:02 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 3089 bytes --]

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
Reported-by: kernel test robot <lkp@intel.com>

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/1] cifs: sanitize multiple delimiters in prepath
  2021-12-15  6:59 ` Leif Sahlberg
@ 2021-12-16  3:14   ` Steve French
  0 siblings, 0 replies; 5+ messages in thread
From: Steve French @ 2021-12-16  3:14 UTC (permalink / raw)
  To: Leif Sahlberg; +Cc: Thiago Rafael Becker, CIFS, Steve French, LKML

[-- Attachment #1: Type: text/plain, Size: 2548 bytes --]

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.

On Wed, Dec 15, 2021 at 1:49 PM Leif Sahlberg <lsahlber@redhat.com> wrote:
>
> Steve,
>
> Acked-by: me
> Steve, we should have this in stable
>
>     Fixes: 24e0a1eff9e2 ("cifs: switch to new mount api")
>     Cc: stable@vger.kernel.org # 5.11+
>
>
> On Wed, Dec 15, 2021 at 1:09 PM Thiago Rafael Becker <trbecker@gmail.com> wrote:
> >
> > mount.cifs can pass a device with multiple delimiters in it. This will
> > cause rename(2) to fail with ENOENT.
> >
> > BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=2031200
> > Signed-off-by: Thiago Rafael Becker <trbecker@gmail.com>
> > ---
> >  fs/cifs/fs_context.c | 30 ++++++++++++++++++++++++++++++
> >  1 file changed, 30 insertions(+)
> >
> > diff --git a/fs/cifs/fs_context.c b/fs/cifs/fs_context.c
> > index 6a179ae753c1..4ce8a7df3a02 100644
> > --- a/fs/cifs/fs_context.c
> > +++ b/fs/cifs/fs_context.c
> > @@ -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
> >
>


-- 
Thanks,

Steve

[-- Attachment #2: 0001-cifs-sanitize-multiple-delimiters-in-prepath.patch --]
[-- Type: text/x-patch, Size: 1979 bytes --]

From 859057cc97cc949b0097cd1e776a6708b367e99c Mon Sep 17 00:00:00 2001
From: Thiago Rafael Becker <trbecker@gmail.com>
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.

BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=2031200
Fixes: 24e0a1eff9e2 ("cifs: switch to new mount api")
Cc: stable@vger.kernel.org # 5.11+
Acked-by: Ronnie Sahlberg <lsahlber@redhat.com>
Signed-off-by: Thiago Rafael Becker <trbecker@gmail.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/cifs/fs_context.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/fs/cifs/fs_context.c b/fs/cifs/fs_context.c
index 6a179ae753c1..4ce8a7df3a02 100644
--- a/fs/cifs/fs_context.c
+++ b/fs/cifs/fs_context.c
@@ -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.32.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-12-16  3:14 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-15  3:08 [PATCH 1/1] cifs: sanitize multiple delimiters in prepath Thiago Rafael Becker
2021-12-15  6:59 ` Leif Sahlberg
2021-12-16  3:14   ` Steve French
2021-12-15  7:55 ` [RFC PATCH] cifs: sanitize_path() can be static kernel test robot
2021-12-15  8:02 ` [PATCH 1/1] cifs: sanitize multiple delimiters in prepath kernel test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.