smb: client: fix use-after-free of signing key

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(-)

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
>