diff mbox series

cifs: fix charset issue in reconnection

Message ID 20230717022227.1736113-1-wentao@uniontech.com
State New
Headers show
Series cifs: fix charset issue in reconnection | expand

Commit Message

Winston Wen July 17, 2023, 2:22 a.m. UTC
We need to specify charset, like "iocharset=utf-8", in mount options for
Chinese path if the nls_default don't support it, such as iso8859-1, the
default value for CONFIG_NLS_DEFAULT.

But now in reconnection the nls_default is used, instead of the one we
specified and used in mount, and this can lead to mount failure.

Signed-off-by: Winston Wen <wentao@uniontech.com>
---
 fs/smb/client/cifsglob.h | 1 +
 fs/smb/client/connect.c  | 6 ++++++
 fs/smb/client/smb2pdu.c  | 3 +--
 3 files changed, 8 insertions(+), 2 deletions(-)

Comments

Paulo Alcantara July 17, 2023, 4 p.m. UTC | #1
Winston Wen <wentao@uniontech.com> writes:

> We need to specify charset, like "iocharset=utf-8", in mount options for
> Chinese path if the nls_default don't support it, such as iso8859-1, the
> default value for CONFIG_NLS_DEFAULT.
>
> But now in reconnection the nls_default is used, instead of the one we
> specified and used in mount, and this can lead to mount failure.
>
> Signed-off-by: Winston Wen <wentao@uniontech.com>
> ---
>  fs/smb/client/cifsglob.h | 1 +
>  fs/smb/client/connect.c  | 6 ++++++
>  fs/smb/client/smb2pdu.c  | 3 +--
>  3 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
> index c9a87d0123ce..31cadf9b2a98 100644
> --- a/fs/smb/client/cifsglob.h
> +++ b/fs/smb/client/cifsglob.h
> @@ -1062,6 +1062,7 @@ struct cifs_ses {
>  	unsigned long chans_need_reconnect;
>  	/* ========= end: protected by chan_lock ======== */
>  	struct cifs_ses *dfs_root_ses;
> +	struct nls_table *local_nls;
>  };
>  
>  static inline bool
> diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
> index 5dd09c6d762e..abb69a6d4fce 100644
> --- a/fs/smb/client/connect.c
> +++ b/fs/smb/client/connect.c
> @@ -1842,6 +1842,10 @@ static int match_session(struct cifs_ses *ses, struct smb3_fs_context *ctx)
>  			    CIFS_MAX_PASSWORD_LEN))
>  			return 0;
>  	}
> +
> +	if (strcmp(ctx->local_nls->charset, ses->local_nls->charset))
> +		return 0;
> +
>  	return 1;
>  }
>  
> @@ -2027,6 +2031,7 @@ void __cifs_put_smb_ses(struct cifs_ses *ses)
>  		}
>  	}
>  
> +	unload_nls(ses->local_nls);

Please move this call to sesInfoFree().

cifs_reconnect_tcon() also needs to be fixed by using @ses->local_nls
rather than load_nls_default().

Otherwise, looks good te me.  Thanks!
Winston Wen July 18, 2023, 1:23 a.m. UTC | #2
On Mon, 17 Jul 2023 13:00:16 -0300
Paulo Alcantara <pc@manguebit.com> wrote:

> Winston Wen <wentao@uniontech.com> writes:
> 
> > We need to specify charset, like "iocharset=utf-8", in mount
> > options for Chinese path if the nls_default don't support it, such
> > as iso8859-1, the default value for CONFIG_NLS_DEFAULT.
> >
> > But now in reconnection the nls_default is used, instead of the one
> > we specified and used in mount, and this can lead to mount failure.
> >
> > Signed-off-by: Winston Wen <wentao@uniontech.com>
> > ---
> >  fs/smb/client/cifsglob.h | 1 +
> >  fs/smb/client/connect.c  | 6 ++++++
> >  fs/smb/client/smb2pdu.c  | 3 +--
> >  3 files changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
> > index c9a87d0123ce..31cadf9b2a98 100644
> > --- a/fs/smb/client/cifsglob.h
> > +++ b/fs/smb/client/cifsglob.h
> > @@ -1062,6 +1062,7 @@ struct cifs_ses {
> >  	unsigned long chans_need_reconnect;
> >  	/* ========= end: protected by chan_lock ======== */
> >  	struct cifs_ses *dfs_root_ses;
> > +	struct nls_table *local_nls;
> >  };
> >  
> >  static inline bool
> > diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
> > index 5dd09c6d762e..abb69a6d4fce 100644
> > --- a/fs/smb/client/connect.c
> > +++ b/fs/smb/client/connect.c
> > @@ -1842,6 +1842,10 @@ static int match_session(struct cifs_ses
> > *ses, struct smb3_fs_context *ctx) CIFS_MAX_PASSWORD_LEN))
> >  			return 0;
> >  	}
> > +
> > +	if (strcmp(ctx->local_nls->charset,
> > ses->local_nls->charset))
> > +		return 0;
> > +
> >  	return 1;
> >  }
> >  
> > @@ -2027,6 +2031,7 @@ void __cifs_put_smb_ses(struct cifs_ses *ses)
> >  		}
> >  	}
> >  
> > +	unload_nls(ses->local_nls);
> 
> Please move this call to sesInfoFree().

Thanks, fixed.

> 
> cifs_reconnect_tcon() also needs to be fixed by using @ses->local_nls
> rather than load_nls_default().

Thanks, fixed. I totally missed this.

> 
> Otherwise, looks good te me.  Thanks!
> 

Thanks for the review!
diff mbox series

Patch

diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
index c9a87d0123ce..31cadf9b2a98 100644
--- a/fs/smb/client/cifsglob.h
+++ b/fs/smb/client/cifsglob.h
@@ -1062,6 +1062,7 @@  struct cifs_ses {
 	unsigned long chans_need_reconnect;
 	/* ========= end: protected by chan_lock ======== */
 	struct cifs_ses *dfs_root_ses;
+	struct nls_table *local_nls;
 };
 
 static inline bool
diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
index 5dd09c6d762e..abb69a6d4fce 100644
--- a/fs/smb/client/connect.c
+++ b/fs/smb/client/connect.c
@@ -1842,6 +1842,10 @@  static int match_session(struct cifs_ses *ses, struct smb3_fs_context *ctx)
 			    CIFS_MAX_PASSWORD_LEN))
 			return 0;
 	}
+
+	if (strcmp(ctx->local_nls->charset, ses->local_nls->charset))
+		return 0;
+
 	return 1;
 }
 
@@ -2027,6 +2031,7 @@  void __cifs_put_smb_ses(struct cifs_ses *ses)
 		}
 	}
 
+	unload_nls(ses->local_nls);
 	sesInfoFree(ses);
 	cifs_put_tcp_session(server, 0);
 }
@@ -2286,6 +2291,7 @@  cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb3_fs_context *ctx)
 
 	ses->sectype = ctx->sectype;
 	ses->sign = ctx->sign;
+	ses->local_nls = load_nls(ctx->local_nls->charset);
 
 	/* add server as first channel */
 	spin_lock(&ses->chan_lock);
diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index e04766fe6f80..a457f07f820d 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -242,7 +242,7 @@  smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
 	}
 	spin_unlock(&server->srv_lock);
 
-	nls_codepage = load_nls_default();
+	nls_codepage = ses->local_nls;
 
 	/*
 	 * need to prevent multiple threads trying to simultaneously
@@ -324,7 +324,6 @@  smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
 		rc = -EAGAIN;
 	}
 failed:
-	unload_nls(nls_codepage);
 	return rc;
 }