diff mbox series

[11/12] smb: client: fix potential UAF in smb2_get_enc_key()

Message ID 20240402193404.236159-11-pc@manguebit.com (mailing list archive)
State New, archived
Headers show
Series [01/12] smb: client: fix potential UAF in cifs_debug_files_proc_show() | expand

Commit Message

Paulo Alcantara April 2, 2024, 7:34 p.m. UTC
Skip sessions that are being teared down (status == SES_EXITING) to
avoid UAF.

Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.com>
---
 fs/smb/client/smb2ops.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Steve French April 2, 2024, 9:40 p.m. UTC | #1
Isn't this needed to send the SMB3 Logoff request?

On Tue, Apr 2, 2024 at 2:35 PM Paulo Alcantara <pc@manguebit.com> wrote:
>
> Skip sessions that are being teared down (status == SES_EXITING) to
> avoid UAF.
>
> Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.com>
> ---
>  fs/smb/client/smb2ops.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
> index 1506a0eb10ba..4fd2ffa2ebba 100644
> --- a/fs/smb/client/smb2ops.c
> +++ b/fs/smb/client/smb2ops.c
> @@ -4188,8 +4188,8 @@ smb2_get_enc_key(struct TCP_Server_Info *server, __u64 ses_id, int enc, u8 *key)
>
>         spin_lock(&cifs_tcp_ses_lock);
>         list_for_each_entry(ses, &pserver->smb_ses_list, smb_ses_list) {
> -               if (ses->Suid == ses_id) {
> -                       spin_lock(&ses->ses_lock);
> +               spin_lock(&ses->ses_lock);
> +               if (ses->ses_status != SES_EXITING && ses->Suid == ses_id) {
>                         ses_enc_key = enc ? ses->smb3encryptionkey :
>                                 ses->smb3decryptionkey;
>                         memcpy(key, ses_enc_key, SMB3_ENC_DEC_KEY_SIZE);
> @@ -4197,6 +4197,7 @@ smb2_get_enc_key(struct TCP_Server_Info *server, __u64 ses_id, int enc, u8 *key)
>                         spin_unlock(&cifs_tcp_ses_lock);
>                         return 0;
>                 }
> +               spin_unlock(&ses->ses_lock);
>         }
>         spin_unlock(&cifs_tcp_ses_lock);
>
> --
> 2.44.0
>
>
Paulo Alcantara April 2, 2024, 9:48 p.m. UTC | #2
Steve French <smfrench@gmail.com> writes:

> Isn't this needed to send the SMB3 Logoff request?

Yes, good catch!  Please drop this one.
Paulo Alcantara April 2, 2024, 10:43 p.m. UTC | #3
Paulo Alcantara <pc@manguebit.com> writes:

> Skip sessions that are being teared down (status == SES_EXITING) to
> avoid UAF.
>
> Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.com>
> ---
>  fs/smb/client/smb2ops.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

As we can send encrypted session logoff when
SMB2_SESSION_FLAG_ENCRYPT_DATA is set, then please ignore this one and
patch 05/12.
diff mbox series

Patch

diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index 1506a0eb10ba..4fd2ffa2ebba 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -4188,8 +4188,8 @@  smb2_get_enc_key(struct TCP_Server_Info *server, __u64 ses_id, int enc, u8 *key)
 
 	spin_lock(&cifs_tcp_ses_lock);
 	list_for_each_entry(ses, &pserver->smb_ses_list, smb_ses_list) {
-		if (ses->Suid == ses_id) {
-			spin_lock(&ses->ses_lock);
+		spin_lock(&ses->ses_lock);
+		if (ses->ses_status != SES_EXITING && ses->Suid == ses_id) {
 			ses_enc_key = enc ? ses->smb3encryptionkey :
 				ses->smb3decryptionkey;
 			memcpy(key, ses_enc_key, SMB3_ENC_DEC_KEY_SIZE);
@@ -4197,6 +4197,7 @@  smb2_get_enc_key(struct TCP_Server_Info *server, __u64 ses_id, int enc, u8 *key)
 			spin_unlock(&cifs_tcp_ses_lock);
 			return 0;
 		}
+		spin_unlock(&ses->ses_lock);
 	}
 	spin_unlock(&cifs_tcp_ses_lock);