* [PATCH] smb: client: fix use-after-free of signing key
@ 2024-11-11 13:40 Paulo Alcantara
2024-11-13 17:04 ` Steve French
0 siblings, 1 reply; 2+ messages in thread
From: Paulo Alcantara @ 2024-11-11 13:40 UTC (permalink / raw)
To: smfrench; +Cc: linux-cifs, Paulo Alcantara, Jay Shin
Customers have reported use-after-free in @ses->auth_key.response with
SMB2.1 + sign mounts which occurs due to following race:
task A task B
cifs_mount()
dfs_mount_share()
get_session()
cifs_mount_get_session() cifs_send_recv()
cifs_get_smb_ses() compound_send_recv()
cifs_setup_session() smb2_setup_request()
kfree_sensitive() smb2_calc_signature()
crypto_shash_setkey() *UAF*
Fix this by ensuring that we have a valid @ses->auth_key.response by
checking whether @ses->ses_status is SES_GOOD or SES_EXITING with
@ses->ses_lock held. After commit 24a9799aa8ef ("smb: client: fix UAF
in smb2_reconnect_server()"), we made sure to call ->logoff() only
when @ses was known to be good (e.g. valid ->auth_key.response), so
it's safe to access signing key when @ses->ses_status == SES_EXITING.
Reported-by: Jay Shin <jaeshin@redhat.com>
Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.com>
---
fs/smb/client/smb2proto.h | 2 --
fs/smb/client/smb2transport.c | 56 +++++++++++++++++++++++++----------
2 files changed, 40 insertions(+), 18 deletions(-)
diff --git a/fs/smb/client/smb2proto.h b/fs/smb/client/smb2proto.h
index 6f9885e4f66c..71504b30909e 100644
--- a/fs/smb/client/smb2proto.h
+++ b/fs/smb/client/smb2proto.h
@@ -37,8 +37,6 @@ extern struct mid_q_entry *smb2_setup_request(struct cifs_ses *ses,
struct smb_rqst *rqst);
extern struct mid_q_entry *smb2_setup_async_request(
struct TCP_Server_Info *server, struct smb_rqst *rqst);
-extern struct cifs_ses *smb2_find_smb_ses(struct TCP_Server_Info *server,
- __u64 ses_id);
extern struct cifs_tcon *smb2_find_smb_tcon(struct TCP_Server_Info *server,
__u64 ses_id, __u32 tid);
extern int smb2_calc_signature(struct smb_rqst *rqst,
diff --git a/fs/smb/client/smb2transport.c b/fs/smb/client/smb2transport.c
index b486b14bb330..475b36c27f65 100644
--- a/fs/smb/client/smb2transport.c
+++ b/fs/smb/client/smb2transport.c
@@ -74,7 +74,7 @@ smb311_crypto_shash_allocate(struct TCP_Server_Info *server)
static
-int smb2_get_sign_key(__u64 ses_id, struct TCP_Server_Info *server, u8 *key)
+int smb3_get_sign_key(__u64 ses_id, struct TCP_Server_Info *server, u8 *key)
{
struct cifs_chan *chan;
struct TCP_Server_Info *pserver;
@@ -168,16 +168,41 @@ smb2_find_smb_ses_unlocked(struct TCP_Server_Info *server, __u64 ses_id)
return NULL;
}
-struct cifs_ses *
-smb2_find_smb_ses(struct TCP_Server_Info *server, __u64 ses_id)
+static int smb2_get_sign_key(struct TCP_Server_Info *server,
+ __u64 ses_id, u8 *key)
{
struct cifs_ses *ses;
+ int rc = -ENOENT;
+
+ if (SERVER_IS_CHAN(server))
+ server = server->primary_server;
spin_lock(&cifs_tcp_ses_lock);
- ses = smb2_find_smb_ses_unlocked(server, ses_id);
+ list_for_each_entry(ses, &server->smb_ses_list, smb_ses_list) {
+ if (ses->Suid != ses_id)
+ continue;
+
+ rc = 0;
+ spin_lock(&ses->ses_lock);
+ switch (ses->ses_status) {
+ case SES_EXITING: /* SMB2_LOGOFF */
+ case SES_GOOD:
+ if (likely(ses->auth_key.response)) {
+ memcpy(key, ses->auth_key.response,
+ SMB2_NTLMV2_SESSKEY_SIZE);
+ } else {
+ rc = -EIO;
+ }
+ break;
+ default:
+ rc = -EAGAIN;
+ break;
+ }
+ spin_unlock(&ses->ses_lock);
+ break;
+ }
spin_unlock(&cifs_tcp_ses_lock);
-
- return ses;
+ return rc;
}
static struct cifs_tcon *
@@ -236,14 +261,16 @@ smb2_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server,
unsigned char *sigptr = smb2_signature;
struct kvec *iov = rqst->rq_iov;
struct smb2_hdr *shdr = (struct smb2_hdr *)iov[0].iov_base;
- struct cifs_ses *ses;
struct shash_desc *shash = NULL;
struct smb_rqst drqst;
+ __u64 sid = le64_to_cpu(shdr->SessionId);
+ u8 key[SMB2_NTLMV2_SESSKEY_SIZE];
- ses = smb2_find_smb_ses(server, le64_to_cpu(shdr->SessionId));
- if (unlikely(!ses)) {
- cifs_server_dbg(FYI, "%s: Could not find session\n", __func__);
- return -ENOENT;
+ rc = smb2_get_sign_key(server, sid, key);
+ if (unlikely(rc)) {
+ cifs_server_dbg(FYI, "%s: [sesid=0x%llx] couldn't find signing key: %d\n",
+ __func__, sid, rc);
+ return rc;
}
memset(smb2_signature, 0x0, SMB2_HMACSHA256_SIZE);
@@ -260,8 +287,7 @@ smb2_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server,
shash = server->secmech.hmacsha256;
}
- rc = crypto_shash_setkey(shash->tfm, ses->auth_key.response,
- SMB2_NTLMV2_SESSKEY_SIZE);
+ rc = crypto_shash_setkey(shash->tfm, key, sizeof(key));
if (rc) {
cifs_server_dbg(VFS,
"%s: Could not update with response\n",
@@ -303,8 +329,6 @@ smb2_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server,
out:
if (allocate_crypto)
cifs_free_hash(&shash);
- if (ses)
- cifs_put_smb_ses(ses);
return rc;
}
@@ -570,7 +594,7 @@ smb3_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server,
struct smb_rqst drqst;
u8 key[SMB3_SIGN_KEY_SIZE];
- rc = smb2_get_sign_key(le64_to_cpu(shdr->SessionId), server, key);
+ rc = smb3_get_sign_key(le64_to_cpu(shdr->SessionId), server, key);
if (unlikely(rc)) {
cifs_server_dbg(FYI, "%s: Could not get signing key\n", __func__);
return rc;
--
2.47.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] smb: client: fix use-after-free of signing key
2024-11-11 13:40 [PATCH] smb: client: fix use-after-free of signing key Paulo Alcantara
@ 2024-11-13 17:04 ` Steve French
0 siblings, 0 replies; 2+ messages in thread
From: Steve French @ 2024-11-13 17:04 UTC (permalink / raw)
To: Paulo Alcantara; +Cc: linux-cifs, Jay Shin
tentatively merged into cifs-2.6.git for-next pending testing and
additional review
On Mon, Nov 11, 2024 at 7:41 AM Paulo Alcantara <pc@manguebit.com> wrote:
>
> Customers have reported use-after-free in @ses->auth_key.response with
> SMB2.1 + sign mounts which occurs due to following race:
>
> task A task B
> cifs_mount()
> dfs_mount_share()
> get_session()
> cifs_mount_get_session() cifs_send_recv()
> cifs_get_smb_ses() compound_send_recv()
> cifs_setup_session() smb2_setup_request()
> kfree_sensitive() smb2_calc_signature()
> crypto_shash_setkey() *UAF*
>
> Fix this by ensuring that we have a valid @ses->auth_key.response by
> checking whether @ses->ses_status is SES_GOOD or SES_EXITING with
> @ses->ses_lock held. After commit 24a9799aa8ef ("smb: client: fix UAF
> in smb2_reconnect_server()"), we made sure to call ->logoff() only
> when @ses was known to be good (e.g. valid ->auth_key.response), so
> it's safe to access signing key when @ses->ses_status == SES_EXITING.
>
> Reported-by: Jay Shin <jaeshin@redhat.com>
> Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.com>
> ---
> fs/smb/client/smb2proto.h | 2 --
> fs/smb/client/smb2transport.c | 56 +++++++++++++++++++++++++----------
> 2 files changed, 40 insertions(+), 18 deletions(-)
>
> diff --git a/fs/smb/client/smb2proto.h b/fs/smb/client/smb2proto.h
> index 6f9885e4f66c..71504b30909e 100644
> --- a/fs/smb/client/smb2proto.h
> +++ b/fs/smb/client/smb2proto.h
> @@ -37,8 +37,6 @@ extern struct mid_q_entry *smb2_setup_request(struct cifs_ses *ses,
> struct smb_rqst *rqst);
> extern struct mid_q_entry *smb2_setup_async_request(
> struct TCP_Server_Info *server, struct smb_rqst *rqst);
> -extern struct cifs_ses *smb2_find_smb_ses(struct TCP_Server_Info *server,
> - __u64 ses_id);
> extern struct cifs_tcon *smb2_find_smb_tcon(struct TCP_Server_Info *server,
> __u64 ses_id, __u32 tid);
> extern int smb2_calc_signature(struct smb_rqst *rqst,
> diff --git a/fs/smb/client/smb2transport.c b/fs/smb/client/smb2transport.c
> index b486b14bb330..475b36c27f65 100644
> --- a/fs/smb/client/smb2transport.c
> +++ b/fs/smb/client/smb2transport.c
> @@ -74,7 +74,7 @@ smb311_crypto_shash_allocate(struct TCP_Server_Info *server)
>
>
> static
> -int smb2_get_sign_key(__u64 ses_id, struct TCP_Server_Info *server, u8 *key)
> +int smb3_get_sign_key(__u64 ses_id, struct TCP_Server_Info *server, u8 *key)
> {
> struct cifs_chan *chan;
> struct TCP_Server_Info *pserver;
> @@ -168,16 +168,41 @@ smb2_find_smb_ses_unlocked(struct TCP_Server_Info *server, __u64 ses_id)
> return NULL;
> }
>
> -struct cifs_ses *
> -smb2_find_smb_ses(struct TCP_Server_Info *server, __u64 ses_id)
> +static int smb2_get_sign_key(struct TCP_Server_Info *server,
> + __u64 ses_id, u8 *key)
> {
> struct cifs_ses *ses;
> + int rc = -ENOENT;
> +
> + if (SERVER_IS_CHAN(server))
> + server = server->primary_server;
>
> spin_lock(&cifs_tcp_ses_lock);
> - ses = smb2_find_smb_ses_unlocked(server, ses_id);
> + list_for_each_entry(ses, &server->smb_ses_list, smb_ses_list) {
> + if (ses->Suid != ses_id)
> + continue;
> +
> + rc = 0;
> + spin_lock(&ses->ses_lock);
> + switch (ses->ses_status) {
> + case SES_EXITING: /* SMB2_LOGOFF */
> + case SES_GOOD:
> + if (likely(ses->auth_key.response)) {
> + memcpy(key, ses->auth_key.response,
> + SMB2_NTLMV2_SESSKEY_SIZE);
> + } else {
> + rc = -EIO;
> + }
> + break;
> + default:
> + rc = -EAGAIN;
> + break;
> + }
> + spin_unlock(&ses->ses_lock);
> + break;
> + }
> spin_unlock(&cifs_tcp_ses_lock);
> -
> - return ses;
> + return rc;
> }
>
> static struct cifs_tcon *
> @@ -236,14 +261,16 @@ smb2_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server,
> unsigned char *sigptr = smb2_signature;
> struct kvec *iov = rqst->rq_iov;
> struct smb2_hdr *shdr = (struct smb2_hdr *)iov[0].iov_base;
> - struct cifs_ses *ses;
> struct shash_desc *shash = NULL;
> struct smb_rqst drqst;
> + __u64 sid = le64_to_cpu(shdr->SessionId);
> + u8 key[SMB2_NTLMV2_SESSKEY_SIZE];
>
> - ses = smb2_find_smb_ses(server, le64_to_cpu(shdr->SessionId));
> - if (unlikely(!ses)) {
> - cifs_server_dbg(FYI, "%s: Could not find session\n", __func__);
> - return -ENOENT;
> + rc = smb2_get_sign_key(server, sid, key);
> + if (unlikely(rc)) {
> + cifs_server_dbg(FYI, "%s: [sesid=0x%llx] couldn't find signing key: %d\n",
> + __func__, sid, rc);
> + return rc;
> }
>
> memset(smb2_signature, 0x0, SMB2_HMACSHA256_SIZE);
> @@ -260,8 +287,7 @@ smb2_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server,
> shash = server->secmech.hmacsha256;
> }
>
> - rc = crypto_shash_setkey(shash->tfm, ses->auth_key.response,
> - SMB2_NTLMV2_SESSKEY_SIZE);
> + rc = crypto_shash_setkey(shash->tfm, key, sizeof(key));
> if (rc) {
> cifs_server_dbg(VFS,
> "%s: Could not update with response\n",
> @@ -303,8 +329,6 @@ smb2_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server,
> out:
> if (allocate_crypto)
> cifs_free_hash(&shash);
> - if (ses)
> - cifs_put_smb_ses(ses);
> return rc;
> }
>
> @@ -570,7 +594,7 @@ smb3_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server,
> struct smb_rqst drqst;
> u8 key[SMB3_SIGN_KEY_SIZE];
>
> - rc = smb2_get_sign_key(le64_to_cpu(shdr->SessionId), server, key);
> + rc = smb3_get_sign_key(le64_to_cpu(shdr->SessionId), server, key);
> if (unlikely(rc)) {
> cifs_server_dbg(FYI, "%s: Could not get signing key\n", __func__);
> return rc;
> --
> 2.47.0
>
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-11-13 17:04 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-11 13:40 [PATCH] smb: client: fix use-after-free of signing key Paulo Alcantara
2024-11-13 17:04 ` Steve French
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox