Skip to content

Commit 0e41457

Browse files
sprasad-microsoftgregkh
authored andcommitted
cifs: during remount, make sure passwords are in sync
[ Upstream commit 0f0e357 ] 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 <[email protected]> Signed-off-by: Meetakshi Setiya <[email protected]> Signed-off-by: Steve French <[email protected]> Signed-off-by: Sasha Levin <[email protected]>
1 parent e70c21d commit 0e41457

2 files changed

Lines changed: 75 additions & 9 deletions

File tree

fs/smb/client/fs_context.c

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

893+
int smb3_sync_session_ctx_passwords(struct cifs_sb_info *cifs_sb, struct cifs_ses *ses)
894+
{
895+
if (ses->password &&
896+
cifs_sb->ctx->password &&
897+
strcmp(ses->password, cifs_sb->ctx->password)) {
898+
kfree_sensitive(cifs_sb->ctx->password);
899+
cifs_sb->ctx->password = kstrdup(ses->password, GFP_KERNEL);
900+
if (!cifs_sb->ctx->password)
901+
return -ENOMEM;
902+
}
903+
if (ses->password2 &&
904+
cifs_sb->ctx->password2 &&
905+
strcmp(ses->password2, cifs_sb->ctx->password2)) {
906+
kfree_sensitive(cifs_sb->ctx->password2);
907+
cifs_sb->ctx->password2 = kstrdup(ses->password2, GFP_KERNEL);
908+
if (!cifs_sb->ctx->password2) {
909+
kfree_sensitive(cifs_sb->ctx->password);
910+
cifs_sb->ctx->password = NULL;
911+
return -ENOMEM;
912+
}
913+
}
914+
return 0;
915+
}
916+
893917
static int smb3_reconfigure(struct fs_context *fc)
894918
{
895919
struct smb3_fs_context *ctx = smb3_fc2context(fc);
896920
struct dentry *root = fc->root;
897921
struct cifs_sb_info *cifs_sb = CIFS_SB(root->d_sb);
898922
struct cifs_ses *ses = cifs_sb_master_tcon(cifs_sb)->ses;
923+
char *new_password = NULL, *new_password2 = NULL;
899924
bool need_recon = false;
900925
int rc;
901926

@@ -915,21 +940,61 @@ static int smb3_reconfigure(struct fs_context *fc)
915940
STEAL_STRING(cifs_sb, ctx, UNC);
916941
STEAL_STRING(cifs_sb, ctx, source);
917942
STEAL_STRING(cifs_sb, ctx, username);
943+
918944
if (need_recon == false)
919945
STEAL_STRING_SENSITIVE(cifs_sb, ctx, password);
920946
else {
921-
kfree_sensitive(ses->password);
922-
ses->password = kstrdup(ctx->password, GFP_KERNEL);
923-
if (!ses->password)
924-
return -ENOMEM;
925-
kfree_sensitive(ses->password2);
926-
ses->password2 = kstrdup(ctx->password2, GFP_KERNEL);
927-
if (!ses->password2) {
928-
kfree_sensitive(ses->password);
929-
ses->password = NULL;
947+
if (ctx->password) {
948+
new_password = kstrdup(ctx->password, GFP_KERNEL);
949+
if (!new_password)
950+
return -ENOMEM;
951+
} else
952+
STEAL_STRING_SENSITIVE(cifs_sb, ctx, password);
953+
}
954+
955+
/*
956+
* if a new password2 has been specified, then reset it's value
957+
* inside the ses struct
958+
*/
959+
if (ctx->password2) {
960+
new_password2 = kstrdup(ctx->password2, GFP_KERNEL);
961+
if (!new_password2) {
962+
kfree_sensitive(new_password);
930963
return -ENOMEM;
931964
}
965+
} else
966+
STEAL_STRING_SENSITIVE(cifs_sb, ctx, password2);
967+
968+
/*
969+
* we may update the passwords in the ses struct below. Make sure we do
970+
* not race with smb2_reconnect
971+
*/
972+
mutex_lock(&ses->session_mutex);
973+
974+
/*
975+
* smb2_reconnect may swap password and password2 in case session setup
976+
* failed. First get ctx passwords in sync with ses passwords. It should
977+
* be okay to do this even if this function were to return an error at a
978+
* later stage
979+
*/
980+
rc = smb3_sync_session_ctx_passwords(cifs_sb, ses);
981+
if (rc)
982+
return rc;
983+
984+
/*
985+
* now that allocations for passwords are done, commit them
986+
*/
987+
if (new_password) {
988+
kfree_sensitive(ses->password);
989+
ses->password = new_password;
932990
}
991+
if (new_password2) {
992+
kfree_sensitive(ses->password2);
993+
ses->password2 = new_password2;
994+
}
995+
996+
mutex_unlock(&ses->session_mutex);
997+
933998
STEAL_STRING(cifs_sb, ctx, domainname);
934999
STEAL_STRING(cifs_sb, ctx, nodename);
9351000
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
@@ -299,6 +299,7 @@ static inline struct smb3_fs_context *smb3_fc2context(const struct fs_context *f
299299
}
300300

301301
extern int smb3_fs_context_dup(struct smb3_fs_context *new_ctx, struct smb3_fs_context *ctx);
302+
extern int smb3_sync_session_ctx_passwords(struct cifs_sb_info *cifs_sb, struct cifs_ses *ses);
302303
extern void smb3_update_mnt_flags(struct cifs_sb_info *cifs_sb);
303304

304305
/*

0 commit comments

Comments
 (0)