Skip to content

Commit a4e430c

Browse files
ematsumiyasmfrench
ematsumiya
authored and
smfrench
committed
cifs: replace kfree() with kfree_sensitive() for sensitive data
Replace kfree with kfree_sensitive, or prepend memzero_explicit() in other cases, when freeing sensitive material that could still be left in memory. Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de> Reported-by: kernel test robot <oliver.sang@intel.com> Link: https://lore.kernel.org/r/202209201529.ec633796-oliver.sang@intel.com Reviewed-by: Paulo Alcantara (SUSE) <pc@cjr.nz> Reviewed-by: Ronnie Sahlberg <lsahlber@redhat.com> Signed-off-by: Steve French <stfrench@microsoft.com>
1 parent f5823f5 commit a4e430c

7 files changed

Lines changed: 52 additions & 29 deletions

File tree

‎fs/cifs/cifsencrypt.c‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -679,7 +679,7 @@ setup_ntlmv2_rsp(struct cifs_ses *ses, const struct nls_table *nls_cp)
679679
unlock:
680680
cifs_server_unlock(ses->server);
681681
setup_ntlmv2_rsp_ret:
682-
kfree(tiblob);
682+
kfree_sensitive(tiblob);
683683

684684
return rc;
685685
}
@@ -753,14 +753,14 @@ cifs_crypto_secmech_release(struct TCP_Server_Info *server)
753753
server->secmech.ccmaesdecrypt = NULL;
754754
}
755755

756-
kfree(server->secmech.sdesccmacaes);
756+
kfree_sensitive(server->secmech.sdesccmacaes);
757757
server->secmech.sdesccmacaes = NULL;
758-
kfree(server->secmech.sdeschmacsha256);
758+
kfree_sensitive(server->secmech.sdeschmacsha256);
759759
server->secmech.sdeschmacsha256 = NULL;
760-
kfree(server->secmech.sdeschmacmd5);
760+
kfree_sensitive(server->secmech.sdeschmacmd5);
761761
server->secmech.sdeschmacmd5 = NULL;
762-
kfree(server->secmech.sdescmd5);
762+
kfree_sensitive(server->secmech.sdescmd5);
763763
server->secmech.sdescmd5 = NULL;
764-
kfree(server->secmech.sdescsha512);
764+
kfree_sensitive(server->secmech.sdescsha512);
765765
server->secmech.sdescsha512 = NULL;
766766
}

‎fs/cifs/connect.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ cifs_abort_connection(struct TCP_Server_Info *server)
311311
}
312312
server->sequence_number = 0;
313313
server->session_estab = false;
314-
kfree(server->session_key.response);
314+
kfree_sensitive(server->session_key.response);
315315
server->session_key.response = NULL;
316316
server->session_key.len = 0;
317317
server->lstrp = jiffies;
@@ -1580,7 +1580,7 @@ cifs_put_tcp_session(struct TCP_Server_Info *server, int from_reconnect)
15801580

15811581
cifs_crypto_secmech_release(server);
15821582

1583-
kfree(server->session_key.response);
1583+
kfree_sensitive(server->session_key.response);
15841584
server->session_key.response = NULL;
15851585
server->session_key.len = 0;
15861586
kfree(server->hostname);
@@ -4135,7 +4135,7 @@ cifs_setup_session(const unsigned int xid, struct cifs_ses *ses,
41354135
if (ses->auth_key.response) {
41364136
cifs_dbg(FYI, "Free previous auth_key.response = %p\n",
41374137
ses->auth_key.response);
4138-
kfree(ses->auth_key.response);
4138+
kfree_sensitive(ses->auth_key.response);
41394139
ses->auth_key.response = NULL;
41404140
ses->auth_key.len = 0;
41414141
}

‎fs/cifs/fs_context.c‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -791,6 +791,13 @@ do { \
791791
cifs_sb->ctx->field = NULL; \
792792
} while (0)
793793

794+
#define STEAL_STRING_SENSITIVE(cifs_sb, ctx, field) \
795+
do { \
796+
kfree_sensitive(ctx->field); \
797+
ctx->field = cifs_sb->ctx->field; \
798+
cifs_sb->ctx->field = NULL; \
799+
} while (0)
800+
794801
static int smb3_reconfigure(struct fs_context *fc)
795802
{
796803
struct smb3_fs_context *ctx = smb3_fc2context(fc);
@@ -811,7 +818,7 @@ static int smb3_reconfigure(struct fs_context *fc)
811818
STEAL_STRING(cifs_sb, ctx, UNC);
812819
STEAL_STRING(cifs_sb, ctx, source);
813820
STEAL_STRING(cifs_sb, ctx, username);
814-
STEAL_STRING(cifs_sb, ctx, password);
821+
STEAL_STRING_SENSITIVE(cifs_sb, ctx, password);
815822
STEAL_STRING(cifs_sb, ctx, domainname);
816823
STEAL_STRING(cifs_sb, ctx, nodename);
817824
STEAL_STRING(cifs_sb, ctx, iocharset);
@@ -1162,7 +1169,7 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
11621169
}
11631170
break;
11641171
case Opt_pass:
1165-
kfree(ctx->password);
1172+
kfree_sensitive(ctx->password);
11661173
ctx->password = NULL;
11671174
if (strlen(param->string) == 0)
11681175
break;
@@ -1470,6 +1477,7 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
14701477
return 0;
14711478

14721479
cifs_parse_mount_err:
1480+
kfree_sensitive(ctx->password);
14731481
return -EINVAL;
14741482
}
14751483

‎fs/cifs/misc.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1119,7 +1119,7 @@ cifs_alloc_hash(const char *name,
11191119
void
11201120
cifs_free_hash(struct crypto_shash **shash, struct sdesc **sdesc)
11211121
{
1122-
kfree(*sdesc);
1122+
kfree_sensitive(*sdesc);
11231123
*sdesc = NULL;
11241124
if (*shash)
11251125
crypto_free_shash(*shash);

‎fs/cifs/sess.c‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1213,6 +1213,12 @@ sess_alloc_buffer(struct sess_data *sess_data, int wct)
12131213
static void
12141214
sess_free_buffer(struct sess_data *sess_data)
12151215
{
1216+
int i;
1217+
1218+
/* zero the session data before freeing, as it might contain sensitive info (keys, etc) */
1219+
for (i = 0; i < 3; i++)
1220+
if (sess_data->iov[i].iov_base)
1221+
memzero_explicit(sess_data->iov[i].iov_base, sess_data->iov[i].iov_len);
12161222

12171223
free_rsp_buf(sess_data->buf0_type, sess_data->iov[0].iov_base);
12181224
sess_data->buf0_type = CIFS_NO_BUFFER;
@@ -1374,7 +1380,7 @@ sess_auth_ntlmv2(struct sess_data *sess_data)
13741380
sess_data->result = rc;
13751381
sess_data->func = NULL;
13761382
sess_free_buffer(sess_data);
1377-
kfree(ses->auth_key.response);
1383+
kfree_sensitive(ses->auth_key.response);
13781384
ses->auth_key.response = NULL;
13791385
}
13801386

@@ -1513,7 +1519,7 @@ sess_auth_kerberos(struct sess_data *sess_data)
15131519
sess_data->result = rc;
15141520
sess_data->func = NULL;
15151521
sess_free_buffer(sess_data);
1516-
kfree(ses->auth_key.response);
1522+
kfree_sensitive(ses->auth_key.response);
15171523
ses->auth_key.response = NULL;
15181524
}
15191525

@@ -1648,7 +1654,7 @@ sess_auth_rawntlmssp_negotiate(struct sess_data *sess_data)
16481654
rc = decode_ntlmssp_challenge(bcc_ptr, blob_len, ses);
16491655

16501656
out_free_ntlmsspblob:
1651-
kfree(ntlmsspblob);
1657+
kfree_sensitive(ntlmsspblob);
16521658
out:
16531659
sess_free_buffer(sess_data);
16541660

@@ -1658,9 +1664,9 @@ sess_auth_rawntlmssp_negotiate(struct sess_data *sess_data)
16581664
}
16591665

16601666
/* Else error. Cleanup */
1661-
kfree(ses->auth_key.response);
1667+
kfree_sensitive(ses->auth_key.response);
16621668
ses->auth_key.response = NULL;
1663-
kfree(ses->ntlmssp);
1669+
kfree_sensitive(ses->ntlmssp);
16641670
ses->ntlmssp = NULL;
16651671

16661672
sess_data->func = NULL;
@@ -1759,17 +1765,17 @@ sess_auth_rawntlmssp_authenticate(struct sess_data *sess_data)
17591765
}
17601766

17611767
out_free_ntlmsspblob:
1762-
kfree(ntlmsspblob);
1768+
kfree_sensitive(ntlmsspblob);
17631769
out:
17641770
sess_free_buffer(sess_data);
17651771

17661772
if (!rc)
17671773
rc = sess_establish_session(sess_data);
17681774

17691775
/* Cleanup */
1770-
kfree(ses->auth_key.response);
1776+
kfree_sensitive(ses->auth_key.response);
17711777
ses->auth_key.response = NULL;
1772-
kfree(ses->ntlmssp);
1778+
kfree_sensitive(ses->ntlmssp);
17731779
ses->ntlmssp = NULL;
17741780

17751781
sess_data->func = NULL;
@@ -1845,7 +1851,7 @@ int CIFS_SessSetup(const unsigned int xid, struct cifs_ses *ses,
18451851
rc = sess_data->result;
18461852

18471853
out:
1848-
kfree(sess_data);
1854+
kfree_sensitive(sess_data);
18491855
return rc;
18501856
}
18511857
#endif /* CONFIG_CIFS_ALLOW_INSECURE_LEGACY */

‎fs/cifs/smb2ops.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4423,11 +4423,11 @@ crypt_message(struct TCP_Server_Info *server, int num_rqst,
44234423
if (!rc && enc)
44244424
memcpy(&tr_hdr->Signature, sign, SMB2_SIGNATURE_SIZE);
44254425

4426-
kfree(iv);
4426+
kfree_sensitive(iv);
44274427
free_sg:
4428-
kfree(sg);
4428+
kfree_sensitive(sg);
44294429
free_req:
4430-
kfree(req);
4430+
kfree_sensitive(req);
44314431
return rc;
44324432
}
44334433

‎fs/cifs/smb2pdu.c‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1345,6 +1345,13 @@ SMB2_sess_alloc_buffer(struct SMB2_sess_data *sess_data)
13451345
static void
13461346
SMB2_sess_free_buffer(struct SMB2_sess_data *sess_data)
13471347
{
1348+
int i;
1349+
1350+
/* zero the session data before freeing, as it might contain sensitive info (keys, etc) */
1351+
for (i = 0; i < 2; i++)
1352+
if (sess_data->iov[i].iov_base)
1353+
memzero_explicit(sess_data->iov[i].iov_base, sess_data->iov[i].iov_len);
1354+
13481355
free_rsp_buf(sess_data->buf0_type, sess_data->iov[0].iov_base);
13491356
sess_data->buf0_type = CIFS_NO_BUFFER;
13501357
}
@@ -1477,6 +1484,8 @@ SMB2_auth_kerberos(struct SMB2_sess_data *sess_data)
14771484
out_put_spnego_key:
14781485
key_invalidate(spnego_key);
14791486
key_put(spnego_key);
1487+
if (rc)
1488+
kfree_sensitive(ses->auth_key.response);
14801489
out:
14811490
sess_data->result = rc;
14821491
sess_data->func = NULL;
@@ -1573,15 +1582,15 @@ SMB2_sess_auth_rawntlmssp_negotiate(struct SMB2_sess_data *sess_data)
15731582
}
15741583

15751584
out:
1576-
kfree(ntlmssp_blob);
1585+
memzero_explicit(ntlmssp_blob, blob_length);
15771586
SMB2_sess_free_buffer(sess_data);
15781587
if (!rc) {
15791588
sess_data->result = 0;
15801589
sess_data->func = SMB2_sess_auth_rawntlmssp_authenticate;
15811590
return;
15821591
}
15831592
out_err:
1584-
kfree(ses->ntlmssp);
1593+
kfree_sensitive(ses->ntlmssp);
15851594
ses->ntlmssp = NULL;
15861595
sess_data->result = rc;
15871596
sess_data->func = NULL;
@@ -1657,9 +1666,9 @@ SMB2_sess_auth_rawntlmssp_authenticate(struct SMB2_sess_data *sess_data)
16571666
}
16581667
#endif
16591668
out:
1660-
kfree(ntlmssp_blob);
1669+
memzero_explicit(ntlmssp_blob, blob_length);
16611670
SMB2_sess_free_buffer(sess_data);
1662-
kfree(ses->ntlmssp);
1671+
kfree_sensitive(ses->ntlmssp);
16631672
ses->ntlmssp = NULL;
16641673
sess_data->result = rc;
16651674
sess_data->func = NULL;
@@ -1737,7 +1746,7 @@ SMB2_sess_setup(const unsigned int xid, struct cifs_ses *ses,
17371746
cifs_server_dbg(VFS, "signing requested but authenticated as guest\n");
17381747
rc = sess_data->result;
17391748
out:
1740-
kfree(sess_data);
1749+
kfree_sensitive(sess_data);
17411750
return rc;
17421751
}
17431752

0 commit comments

Comments
 (0)