Skip to content

Commit 0f0e357

Browse files
sprasad-microsoftSteve French
authored andcommitted
cifs: during remount, make sure passwords are in sync
This fixes scenarios where remount can overwrite the only currently working password, breaking reconnect. 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: 35f8342 ("smb3: fix broken reconnect when password changing on the server by allowing password rotation") Signed-off-by: Shyam Prasad N <sprasad@microsoft.com> Signed-off-by: Meetakshi Setiya <msetiya@microsoft.com> Signed-off-by: Steve French <stfrench@microsoft.com>
1 parent b9aef1b commit 0f0e357

File tree

2 files changed

+75
-9
lines changed

2 files changed

+75
-9
lines changed

fs/smb/client/fs_context.c

Lines changed: 74 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -920,12 +920,37 @@ do { \
920920
cifs_sb->ctx->field = NULL; \
921921
} while (0)
922922

923+
int smb3_sync_session_ctx_passwords(struct cifs_sb_info *cifs_sb, struct cifs_ses *ses)
924+
{
925+
if (ses->password &&
926+
cifs_sb->ctx->password &&
927+
strcmp(ses->password, cifs_sb->ctx->password)) {
928+
kfree_sensitive(cifs_sb->ctx->password);
929+
cifs_sb->ctx->password = kstrdup(ses->password, GFP_KERNEL);
930+
if (!cifs_sb->ctx->password)
931+
return -ENOMEM;
932+
}
933+
if (ses->password2 &&
934+
cifs_sb->ctx->password2 &&
935+
strcmp(ses->password2, cifs_sb->ctx->password2)) {
936+
kfree_sensitive(cifs_sb->ctx->password2);
937+
cifs_sb->ctx->password2 = kstrdup(ses->password2, GFP_KERNEL);
938+
if (!cifs_sb->ctx->password2) {
939+
kfree_sensitive(cifs_sb->ctx->password);
940+
cifs_sb->ctx->password = NULL;
941+
return -ENOMEM;
942+
}
943+
}
944+
return 0;
945+
}
946+
923947
static int smb3_reconfigure(struct fs_context *fc)
924948
{
925949
struct smb3_fs_context *ctx = smb3_fc2context(fc);
926950
struct dentry *root = fc->root;
927951
struct cifs_sb_info *cifs_sb = CIFS_SB(root->d_sb);
928952
struct cifs_ses *ses = cifs_sb_master_tcon(cifs_sb)->ses;
953+
char *new_password = NULL, *new_password2 = NULL;
929954
bool need_recon = false;
930955
int rc;
931956

@@ -945,21 +970,61 @@ static int smb3_reconfigure(struct fs_context *fc)
945970
STEAL_STRING(cifs_sb, ctx, UNC);
946971
STEAL_STRING(cifs_sb, ctx, source);
947972
STEAL_STRING(cifs_sb, ctx, username);
973+
948974
if (need_recon == false)
949975
STEAL_STRING_SENSITIVE(cifs_sb, ctx, password);
950976
else {
951-
kfree_sensitive(ses->password);
952-
ses->password = kstrdup(ctx->password, GFP_KERNEL);
953-
if (!ses->password)
954-
return -ENOMEM;
955-
kfree_sensitive(ses->password2);
956-
ses->password2 = kstrdup(ctx->password2, GFP_KERNEL);
957-
if (!ses->password2) {
958-
kfree_sensitive(ses->password);
959-
ses->password = NULL;
977+
if (ctx->password) {
978+
new_password = kstrdup(ctx->password, GFP_KERNEL);
979+
if (!new_password)
980+
return -ENOMEM;
981+
} else
982+
STEAL_STRING_SENSITIVE(cifs_sb, ctx, password);
983+
}
984+
985+
/*
986+
* if a new password2 has been specified, then reset it's value
987+
* inside the ses struct
988+
*/
989+
if (ctx->password2) {
990+
new_password2 = kstrdup(ctx->password2, GFP_KERNEL);
991+
if (!new_password2) {
992+
kfree_sensitive(new_password);
960993
return -ENOMEM;
961994
}
995+
} else
996+
STEAL_STRING_SENSITIVE(cifs_sb, ctx, password2);
997+
998+
/*
999+
* we may update the passwords in the ses struct below. Make sure we do
1000+
* not race with smb2_reconnect
1001+
*/
1002+
mutex_lock(&ses->session_mutex);
1003+
1004+
/*
1005+
* smb2_reconnect may swap password and password2 in case session setup
1006+
* failed. First get ctx passwords in sync with ses passwords. It should
1007+
* be okay to do this even if this function were to return an error at a
1008+
* later stage
1009+
*/
1010+
rc = smb3_sync_session_ctx_passwords(cifs_sb, ses);
1011+
if (rc)
1012+
return rc;
1013+
1014+
/*
1015+
* now that allocations for passwords are done, commit them
1016+
*/
1017+
if (new_password) {
1018+
kfree_sensitive(ses->password);
1019+
ses->password = new_password;
9621020
}
1021+
if (new_password2) {
1022+
kfree_sensitive(ses->password2);
1023+
ses->password2 = new_password2;
1024+
}
1025+
1026+
mutex_unlock(&ses->session_mutex);
1027+
9631028
STEAL_STRING(cifs_sb, ctx, domainname);
9641029
STEAL_STRING(cifs_sb, ctx, nodename);
9651030
STEAL_STRING(cifs_sb, ctx, iocharset);

fs/smb/client/fs_context.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,7 @@ static inline struct smb3_fs_context *smb3_fc2context(const struct fs_context *f
309309
}
310310

311311
extern int smb3_fs_context_dup(struct smb3_fs_context *new_ctx, struct smb3_fs_context *ctx);
312+
extern int smb3_sync_session_ctx_passwords(struct cifs_sb_info *cifs_sb, struct cifs_ses *ses);
312313
extern void smb3_update_mnt_flags(struct cifs_sb_info *cifs_sb);
313314

314315
/*

0 commit comments

Comments
 (0)