From: Shyam Prasad N <sprasad@xxxxxxxxxxxxx>
We recently introduced a password2 field in both ses and ctx structs.
This was done so as to allow the client to rotate passwords for a mount
without any downtime. However, when the client transparently handles
password rotation, it can swap the values of the two password fields
in the ses struct, but not in smb3_fs_context struct that hangs off
cifs_sb. This can lead to a situation where a remount unintentionally
overwrites a working password in the ses struct.
In order to fix this, we first get the passwords in ctx struct
in-sync with ses struct, before replacing them with what the passwords
that could be passed as a part of remount.
Also, in order to avoid race condition between smb2_reconnect and
smb3_reconfigure, we make sure to lock session_mutex before changing
password and password2 fields of the ses structure.
Fixes: 35f834265e0d ("smb3: fix broken reconnect when password changing on the server by allowing password rotation")
Signed-off-by: Shyam Prasad N <sprasad@xxxxxxxxxxxxx>
Signed-off-by: Meetakshi Setiya <msetiya@xxxxxxxxxxxxx>
---
fs/smb/client/fs_context.c | 69 +++++++++++++++++++++++++++++++++-----
1 file changed, 60 insertions(+), 9 deletions(-)
diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c
index 5c5a52019efa..73610e66c8d9 100644
--- a/fs/smb/client/fs_context.c
+++ b/fs/smb/client/fs_context.c
@@ -896,6 +896,7 @@ static int smb3_reconfigure(struct fs_context *fc)
struct dentry *root = fc->root;
struct cifs_sb_info *cifs_sb = CIFS_SB(root->d_sb);
struct cifs_ses *ses = cifs_sb_master_tcon(cifs_sb)->ses;
+ char *new_password = NULL, *new_password2 = NULL;
bool need_recon = false;
int rc;
@@ -915,21 +916,71 @@ static int smb3_reconfigure(struct fs_context *fc)
STEAL_STRING(cifs_sb, ctx, UNC);
STEAL_STRING(cifs_sb, ctx, source);
STEAL_STRING(cifs_sb, ctx, username);
+
if (need_recon == false)
STEAL_STRING_SENSITIVE(cifs_sb, ctx, password);
else {
- kfree_sensitive(ses->password);
- ses->password = kstrdup(ctx->password, GFP_KERNEL);
- if (!ses->password)
- return -ENOMEM;
- kfree_sensitive(ses->password2);
- ses->password2 = kstrdup(ctx->password2, GFP_KERNEL);
- if (!ses->password2) {
- kfree_sensitive(ses->password);
- ses->password = NULL;
+ if (ctx->password) {
+ new_password = kstrdup(ctx->password, GFP_KERNEL);
+ if (!new_password)
+ return -ENOMEM;
+ } else
+ STEAL_STRING_SENSITIVE(cifs_sb, ctx, password);
+ }
+
+ /*
+ * if a new password2 has been specified, then reset it's value
+ * inside the ses struct
+ */
+ if (ctx->password2) {
+ new_password2 = kstrdup(ctx->password2, GFP_KERNEL);
+ if (!new_password2) {
+ if (new_password)
+ kfree_sensitive(new_password);
return -ENOMEM;
}
+ } else
+ STEAL_STRING_SENSITIVE(cifs_sb, ctx, password2);
+
+ /*
+ * we may update the passwords in the ses struct below. Make sure we do
+ * not race with smb2_reconnect
+ */
+ mutex_lock(&ses->session_mutex);
+
+ /*
+ * smb2_reconnect may swap password and password2 in case session setup
+ * failed. First get ctx passwords in sync with ses passwords. It should
+ * be okay to do this even if this function were to return an error at a
+ * later stage
+ */
+ if (ses->password &&
+ cifs_sb->ctx->password &&
+ strcmp(ses->password, cifs_sb->ctx->password)) {
+ kfree_sensitive(cifs_sb->ctx->password);
+ cifs_sb->ctx->password = kstrdup(ses->password, GFP_KERNEL);
+ }
+ if (ses->password2 &&
+ cifs_sb->ctx->password2 &&
+ strcmp(ses->password2, cifs_sb->ctx->password2)) {
+ kfree_sensitive(cifs_sb->ctx->password2);
+ cifs_sb->ctx->password2 = kstrdup(ses->password2, GFP_KERNEL);
+ }
+
+ /*
+ * now that allocations for passwords are done, commit them
+ */
+ if (new_password) {
+ kfree_sensitive(ses->password);
+ ses->password = new_password;
+ }
+ if (new_password2) {
+ kfree_sensitive(ses->password2);
+ ses->password2 = new_password2;
}
+
+ mutex_unlock(&ses->session_mutex);
+
STEAL_STRING(cifs_sb, ctx, domainname);
STEAL_STRING(cifs_sb, ctx, nodename);
STEAL_STRING(cifs_sb, ctx, iocharset);
--
2.46.0.46.g406f326d27
