Skip to content

Commit 1cca91e

Browse files
committed
tls: fix handling of zero-length records on the rx_list
jira VULN-136507 cve CVE-2025-39682 commit-author Jakub Kicinski <kuba@kernel.org> commit 62708b9 Each recvmsg() call must process either - only contiguous DATA records (any number of them) - one non-DATA record If the next record has different type than what has already been processed we break out of the main processing loop. If the record has already been decrypted (which may be the case for TLS 1.3 where we don't know type until decryption) we queue the pending record to the rx_list. Next recvmsg() will pick it up from there. Queuing the skb to rx_list after zero-copy decrypt is not possible, since in that case we decrypted directly to the user space buffer, and we don't have an skb to queue (darg.skb points to the ciphertext skb for access to metadata like length). Only data records are allowed zero-copy, and we break the processing loop after each non-data record. So we should never zero-copy and then find out that the record type has changed. The corner case we missed is when the initial record comes from rx_list, and it's zero length. Reported-by: Muhammad Alifa Ramdhan <ramdhan@starlabs.sg> Reported-by: Billy Jheng Bing-Jhong <billy@starlabs.sg> Fixes: 84c61fe ("tls: rx: do not use the standard strparser") Reviewed-by: Sabrina Dubroca <sd@queasysnail.net> Link: https://patch.msgid.link/20250820021952.143068-1-kuba@kernel.org Signed-off-by: Jakub Kicinski <kuba@kernel.org> (cherry picked from commit 62708b9) Signed-off-by: Brett Mastbergen <bmastbergen@ciq.com>
1 parent 78aa598 commit 1cca91e

File tree

8 files changed

+58
-30
lines changed

8 files changed

+58
-30
lines changed

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
@@ -288,7 +288,7 @@ cifs_abort_connection(struct TCP_Server_Info *server)
288288
}
289289
server->sequence_number = 0;
290290
server->session_estab = false;
291-
kfree(server->session_key.response);
291+
kfree_sensitive(server->session_key.response);
292292
server->session_key.response = NULL;
293293
server->session_key.len = 0;
294294
server->lstrp = jiffies;
@@ -1562,7 +1562,7 @@ cifs_put_tcp_session(struct TCP_Server_Info *server, int from_reconnect)
15621562

15631563
cifs_crypto_secmech_release(server);
15641564

1565-
kfree(server->session_key.response);
1565+
kfree_sensitive(server->session_key.response);
15661566
server->session_key.response = NULL;
15671567
server->session_key.len = 0;
15681568
kfree(server->hostname);
@@ -4097,7 +4097,7 @@ cifs_setup_session(const unsigned int xid, struct cifs_ses *ses,
40974097
if (ses->auth_key.response) {
40984098
cifs_dbg(FYI, "Free previous auth_key.response = %p\n",
40994099
ses->auth_key.response);
4100-
kfree(ses->auth_key.response);
4100+
kfree_sensitive(ses->auth_key.response);
41014101
ses->auth_key.response = NULL;
41024102
ses->auth_key.len = 0;
41034103
}

fs/cifs/fs_context.c

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

792+
#define STEAL_STRING_SENSITIVE(cifs_sb, ctx, field) \
793+
do { \
794+
kfree_sensitive(ctx->field); \
795+
ctx->field = cifs_sb->ctx->field; \
796+
cifs_sb->ctx->field = NULL; \
797+
} while (0)
798+
792799
static int smb3_reconfigure(struct fs_context *fc)
793800
{
794801
struct smb3_fs_context *ctx = smb3_fc2context(fc);
@@ -809,7 +816,7 @@ static int smb3_reconfigure(struct fs_context *fc)
809816
STEAL_STRING(cifs_sb, ctx, UNC);
810817
STEAL_STRING(cifs_sb, ctx, source);
811818
STEAL_STRING(cifs_sb, ctx, username);
812-
STEAL_STRING(cifs_sb, ctx, password);
819+
STEAL_STRING_SENSITIVE(cifs_sb, ctx, password);
813820
STEAL_STRING(cifs_sb, ctx, domainname);
814821
STEAL_STRING(cifs_sb, ctx, nodename);
815822
STEAL_STRING(cifs_sb, ctx, iocharset);
@@ -1150,7 +1157,7 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
11501157
}
11511158
break;
11521159
case Opt_pass:
1153-
kfree(ctx->password);
1160+
kfree_sensitive(ctx->password);
11541161
ctx->password = NULL;
11551162
if (strlen(param->string) == 0)
11561163
break;
@@ -1458,6 +1465,7 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
14581465
return 0;
14591466

14601467
cifs_parse_mount_err:
1468+
kfree_sensitive(ctx->password);
14611469
return -EINVAL;
14621470
}
14631471

fs/cifs/misc.c

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

fs/cifs/sess.c

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

12151221
free_rsp_buf(sess_data->buf0_type, sess_data->iov[0].iov_base);
12161222
sess_data->buf0_type = CIFS_NO_BUFFER;
@@ -1372,7 +1378,7 @@ sess_auth_ntlmv2(struct sess_data *sess_data)
13721378
sess_data->result = rc;
13731379
sess_data->func = NULL;
13741380
sess_free_buffer(sess_data);
1375-
kfree(ses->auth_key.response);
1381+
kfree_sensitive(ses->auth_key.response);
13761382
ses->auth_key.response = NULL;
13771383
}
13781384

@@ -1511,7 +1517,7 @@ sess_auth_kerberos(struct sess_data *sess_data)
15111517
sess_data->result = rc;
15121518
sess_data->func = NULL;
15131519
sess_free_buffer(sess_data);
1514-
kfree(ses->auth_key.response);
1520+
kfree_sensitive(ses->auth_key.response);
15151521
ses->auth_key.response = NULL;
15161522
}
15171523

@@ -1646,7 +1652,7 @@ sess_auth_rawntlmssp_negotiate(struct sess_data *sess_data)
16461652
rc = decode_ntlmssp_challenge(bcc_ptr, blob_len, ses);
16471653

16481654
out_free_ntlmsspblob:
1649-
kfree(ntlmsspblob);
1655+
kfree_sensitive(ntlmsspblob);
16501656
out:
16511657
sess_free_buffer(sess_data);
16521658

@@ -1656,9 +1662,9 @@ sess_auth_rawntlmssp_negotiate(struct sess_data *sess_data)
16561662
}
16571663

16581664
/* Else error. Cleanup */
1659-
kfree(ses->auth_key.response);
1665+
kfree_sensitive(ses->auth_key.response);
16601666
ses->auth_key.response = NULL;
1661-
kfree(ses->ntlmssp);
1667+
kfree_sensitive(ses->ntlmssp);
16621668
ses->ntlmssp = NULL;
16631669

16641670
sess_data->func = NULL;
@@ -1757,17 +1763,17 @@ sess_auth_rawntlmssp_authenticate(struct sess_data *sess_data)
17571763
}
17581764

17591765
out_free_ntlmsspblob:
1760-
kfree(ntlmsspblob);
1766+
kfree_sensitive(ntlmsspblob);
17611767
out:
17621768
sess_free_buffer(sess_data);
17631769

17641770
if (!rc)
17651771
rc = sess_establish_session(sess_data);
17661772

17671773
/* Cleanup */
1768-
kfree(ses->auth_key.response);
1774+
kfree_sensitive(ses->auth_key.response);
17691775
ses->auth_key.response = NULL;
1770-
kfree(ses->ntlmssp);
1776+
kfree_sensitive(ses->ntlmssp);
17711777
ses->ntlmssp = NULL;
17721778

17731779
sess_data->func = NULL;
@@ -1843,7 +1849,7 @@ int CIFS_SessSetup(const unsigned int xid, struct cifs_ses *ses,
18431849
rc = sess_data->result;
18441850

18451851
out:
1846-
kfree(sess_data);
1852+
kfree_sensitive(sess_data);
18471853
return rc;
18481854
}
18491855
#endif /* CONFIG_CIFS_ALLOW_INSECURE_LEGACY */

fs/cifs/smb2ops.c

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

4393-
kfree(iv);
4393+
kfree_sensitive(iv);
43944394
free_sg:
4395-
kfree(sg);
4395+
kfree_sensitive(sg);
43964396
free_req:
4397-
kfree(req);
4397+
kfree_sensitive(req);
43984398
return rc;
43994399
}
44004400

fs/cifs/smb2pdu.c

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1333,6 +1333,13 @@ SMB2_sess_alloc_buffer(struct SMB2_sess_data *sess_data)
13331333
static void
13341334
SMB2_sess_free_buffer(struct SMB2_sess_data *sess_data)
13351335
{
1336+
int i;
1337+
1338+
/* zero the session data before freeing, as it might contain sensitive info (keys, etc) */
1339+
for (i = 0; i < 2; i++)
1340+
if (sess_data->iov[i].iov_base)
1341+
memzero_explicit(sess_data->iov[i].iov_base, sess_data->iov[i].iov_len);
1342+
13361343
free_rsp_buf(sess_data->buf0_type, sess_data->iov[0].iov_base);
13371344
sess_data->buf0_type = CIFS_NO_BUFFER;
13381345
}
@@ -1465,6 +1472,8 @@ SMB2_auth_kerberos(struct SMB2_sess_data *sess_data)
14651472
out_put_spnego_key:
14661473
key_invalidate(spnego_key);
14671474
key_put(spnego_key);
1475+
if (rc)
1476+
kfree_sensitive(ses->auth_key.response);
14681477
out:
14691478
sess_data->result = rc;
14701479
sess_data->func = NULL;
@@ -1561,15 +1570,15 @@ SMB2_sess_auth_rawntlmssp_negotiate(struct SMB2_sess_data *sess_data)
15611570
}
15621571

15631572
out:
1564-
kfree(ntlmssp_blob);
1573+
memzero_explicit(ntlmssp_blob, blob_length);
15651574
SMB2_sess_free_buffer(sess_data);
15661575
if (!rc) {
15671576
sess_data->result = 0;
15681577
sess_data->func = SMB2_sess_auth_rawntlmssp_authenticate;
15691578
return;
15701579
}
15711580
out_err:
1572-
kfree(ses->ntlmssp);
1581+
kfree_sensitive(ses->ntlmssp);
15731582
ses->ntlmssp = NULL;
15741583
sess_data->result = rc;
15751584
sess_data->func = NULL;
@@ -1645,9 +1654,9 @@ SMB2_sess_auth_rawntlmssp_authenticate(struct SMB2_sess_data *sess_data)
16451654
}
16461655
#endif
16471656
out:
1648-
kfree(ntlmssp_blob);
1657+
memzero_explicit(ntlmssp_blob, blob_length);
16491658
SMB2_sess_free_buffer(sess_data);
1650-
kfree(ses->ntlmssp);
1659+
kfree_sensitive(ses->ntlmssp);
16511660
ses->ntlmssp = NULL;
16521661
sess_data->result = rc;
16531662
sess_data->func = NULL;
@@ -1725,7 +1734,7 @@ SMB2_sess_setup(const unsigned int xid, struct cifs_ses *ses,
17251734
cifs_server_dbg(VFS, "signing requested but authenticated as guest\n");
17261735
rc = sess_data->result;
17271736
out:
1728-
kfree(sess_data);
1737+
kfree_sensitive(sess_data);
17291738
return rc;
17301739
}
17311740

net/tls/tls_sw.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1713,6 +1713,9 @@ int decrypt_skb(struct sock *sk, struct scatterlist *sgout)
17131713
return tls_decrypt_sg(sk, NULL, sgout, &darg);
17141714
}
17151715

1716+
/* All records returned from a recvmsg() call must have the same type.
1717+
* 0 is not a valid content type. Use it as "no type reported, yet".
1718+
*/
17161719
static int tls_record_content_type(struct msghdr *msg, struct tls_msg *tlm,
17171720
u8 *control)
17181721
{
@@ -1949,8 +1952,10 @@ int tls_sw_recvmsg(struct sock *sk,
19491952
if (err < 0)
19501953
goto end;
19511954

1955+
/* process_rx_list() will set @control if it processed any records */
19521956
copied = err;
1953-
if (len <= copied || (copied && control != TLS_RECORD_TYPE_DATA) || rx_more)
1957+
if (len <= copied || rx_more ||
1958+
(control && control != TLS_RECORD_TYPE_DATA))
19541959
goto end;
19551960

19561961
target = sock_rcvlowat(sk, flags & MSG_WAITALL, len);

0 commit comments

Comments
 (0)