diff mbox series

cifs: fix handling of escaped ',' in the password mount argument

Message ID 20210225073627.32234-1-lsahlber@redhat.com
State New
Headers show
Series cifs: fix handling of escaped ',' in the password mount argument | expand

Commit Message

Ronnie Sahlberg Feb. 25, 2021, 7:36 a.m. UTC
Passwords can contain ',' which are also used as the separator between
mount options. Mount.cifs will escape all ',' characters as the string ",,".
Update parsing of the mount options to detect ",," and treat it as a single
'c' character.

Fixes: 24e0a1eff9e2 ("cifs: switch to new mount api")
Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
---
 fs/cifs/fs_context.c | 43 ++++++++++++++++++++++++++++++-------------
 1 file changed, 30 insertions(+), 13 deletions(-)

Comments

Steve French Feb. 25, 2021, 6:26 p.m. UTC | #1
added cc: stable # 5.11 and pushed into cifs-2.6 for-next

On Thu, Feb 25, 2021 at 1:36 AM Ronnie Sahlberg <lsahlber@redhat.com> wrote:
>
> Passwords can contain ',' which are also used as the separator between
> mount options. Mount.cifs will escape all ',' characters as the string ",,".
> Update parsing of the mount options to detect ",," and treat it as a single
> 'c' character.
>
> Fixes: 24e0a1eff9e2 ("cifs: switch to new mount api")
> Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
> ---
>  fs/cifs/fs_context.c | 43 ++++++++++++++++++++++++++++++-------------
>  1 file changed, 30 insertions(+), 13 deletions(-)
>
> diff --git a/fs/cifs/fs_context.c b/fs/cifs/fs_context.c
> index 14c955a30006..892f51a21278 100644
> --- a/fs/cifs/fs_context.c
> +++ b/fs/cifs/fs_context.c
> @@ -544,20 +544,37 @@ static int smb3_fs_context_parse_monolithic(struct fs_context *fc,
>
>         /* BB Need to add support for sep= here TBD */
>         while ((key = strsep(&options, ",")) != NULL) {
> -               if (*key) {
> -                       size_t v_len = 0;
> -                       char *value = strchr(key, '=');
> -
> -                       if (value) {
> -                               if (value == key)
> -                                       continue;
> -                               *value++ = 0;
> -                               v_len = strlen(value);
> -                       }
> -                       ret = vfs_parse_fs_string(fc, key, value, v_len);
> -                       if (ret < 0)
> -                               break;
> +               size_t len;
> +               char *value;
> +
> +               if (*key == 0)
> +                       break;
> +
> +               /* Check if following character is the deliminator If yes,
> +                * we have encountered a double deliminator reset the NULL
> +                * character to the deliminator
> +                */
> +               while (options && options[0] == ',') {
> +                       len = strlen(key);
> +                       strcpy(key + len, options);
> +                       options = strchr(options, ',');
> +                       if (options)
> +                               *options++ = 0;
>                 }
> +
> +
> +               len = 0;
> +               value = strchr(key, '=');
> +               if (value) {
> +                       if (value == key)
> +                               continue;
> +                       *value++ = 0;
> +                       len = strlen(value);
> +               }
> +
> +               ret = vfs_parse_fs_string(fc, key, value, len);
> +               if (ret < 0)
> +                       break;
>         }
>
>         return ret;
> --
> 2.13.6
>
Steve French Feb. 25, 2021, 6:27 p.m. UTC | #2
And also added Simon's Reported-by and Tested-by

On Thu, Feb 25, 2021 at 12:26 PM Steve French <smfrench@gmail.com> wrote:
>
> added cc: stable # 5.11 and pushed into cifs-2.6 for-next
>
> On Thu, Feb 25, 2021 at 1:36 AM Ronnie Sahlberg <lsahlber@redhat.com> wrote:
> >
> > Passwords can contain ',' which are also used as the separator between
> > mount options. Mount.cifs will escape all ',' characters as the string ",,".
> > Update parsing of the mount options to detect ",," and treat it as a single
> > 'c' character.
> >
> > Fixes: 24e0a1eff9e2 ("cifs: switch to new mount api")
> > Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
> > ---
> >  fs/cifs/fs_context.c | 43 ++++++++++++++++++++++++++++++-------------
> >  1 file changed, 30 insertions(+), 13 deletions(-)
> >
> > diff --git a/fs/cifs/fs_context.c b/fs/cifs/fs_context.c
> > index 14c955a30006..892f51a21278 100644
> > --- a/fs/cifs/fs_context.c
> > +++ b/fs/cifs/fs_context.c
> > @@ -544,20 +544,37 @@ static int smb3_fs_context_parse_monolithic(struct fs_context *fc,
> >
> >         /* BB Need to add support for sep= here TBD */
> >         while ((key = strsep(&options, ",")) != NULL) {
> > -               if (*key) {
> > -                       size_t v_len = 0;
> > -                       char *value = strchr(key, '=');
> > -
> > -                       if (value) {
> > -                               if (value == key)
> > -                                       continue;
> > -                               *value++ = 0;
> > -                               v_len = strlen(value);
> > -                       }
> > -                       ret = vfs_parse_fs_string(fc, key, value, v_len);
> > -                       if (ret < 0)
> > -                               break;
> > +               size_t len;
> > +               char *value;
> > +
> > +               if (*key == 0)
> > +                       break;
> > +
> > +               /* Check if following character is the deliminator If yes,
> > +                * we have encountered a double deliminator reset the NULL
> > +                * character to the deliminator
> > +                */
> > +               while (options && options[0] == ',') {
> > +                       len = strlen(key);
> > +                       strcpy(key + len, options);
> > +                       options = strchr(options, ',');
> > +                       if (options)
> > +                               *options++ = 0;
> >                 }
> > +
> > +
> > +               len = 0;
> > +               value = strchr(key, '=');
> > +               if (value) {
> > +                       if (value == key)
> > +                               continue;
> > +                       *value++ = 0;
> > +                       len = strlen(value);
> > +               }
> > +
> > +               ret = vfs_parse_fs_string(fc, key, value, len);
> > +               if (ret < 0)
> > +                       break;
> >         }
> >
> >         return ret;
> > --
> > 2.13.6
> >
>
>
> --
> Thanks,
>
> Steve
diff mbox series

Patch

diff --git a/fs/cifs/fs_context.c b/fs/cifs/fs_context.c
index 14c955a30006..892f51a21278 100644
--- a/fs/cifs/fs_context.c
+++ b/fs/cifs/fs_context.c
@@ -544,20 +544,37 @@  static int smb3_fs_context_parse_monolithic(struct fs_context *fc,
 
 	/* BB Need to add support for sep= here TBD */
 	while ((key = strsep(&options, ",")) != NULL) {
-		if (*key) {
-			size_t v_len = 0;
-			char *value = strchr(key, '=');
-
-			if (value) {
-				if (value == key)
-					continue;
-				*value++ = 0;
-				v_len = strlen(value);
-			}
-			ret = vfs_parse_fs_string(fc, key, value, v_len);
-			if (ret < 0)
-				break;
+		size_t len;
+		char *value;
+
+		if (*key == 0)
+			break;
+
+		/* Check if following character is the deliminator If yes,
+		 * we have encountered a double deliminator reset the NULL
+		 * character to the deliminator
+		 */
+		while (options && options[0] == ',') {
+			len = strlen(key);
+			strcpy(key + len, options);
+			options = strchr(options, ',');
+			if (options)
+				*options++ = 0;
 		}
+
+
+		len = 0;
+		value = strchr(key, '=');
+		if (value) {
+			if (value == key)
+				continue;
+			*value++ = 0;
+			len = strlen(value);
+		}
+
+		ret = vfs_parse_fs_string(fc, key, value, len);
+		if (ret < 0)
+			break;
 	}
 
 	return ret;