[PATCH] cifs: Remove locking in smb2_verify_signature() when calculating SMB2/SMB3 signature on receiving packets

Long Li longli at microsoft.com
Thu Mar 26 01:57:26 UTC 2020


>Subject: Re: [PATCH] cifs: Remove locking in smb2_verify_signature() when
>calculating SMB2/SMB3 signature on receiving packets
>
>Wonder if we should add a separate documentation patch to cifsglob.h to
>extend the descriptions of what the various locks and semaphores are
>supposed to protect to include what you noticed?  See below

Yes, I will send this patch.

>
> *  Locking notes.  All updates to global variables and lists should be
> *                  protected by spinlocks or semaphores.
> *
> *  Spinlocks
> *  ---------
> *  GlobalMid_Lock protects:
> *      list operations on pending_mid_q and oplockQ
> *      updates to XID counters, multiplex id  and SMB sequence numbers
> *      list operations on global DnotifyReqList
> *  tcp_ses_lock protects:
> *      list operations on tcp and SMB session lists
> *  tcon->open_file_lock protects the list of open files hanging off the tcon
> *  inode->open_file_lock protects the openFileList hanging off the inode
> *  cfile->file_info_lock protects counters and fields in cifs file struct
> *  f_owner.lock protects certain per file struct operations
> *  mapping->page_lock protects certain per page operations
> *
> *  Note that the cifs_tcon.open_file_lock should be taken before
> *  not after the cifsInodeInfo.open_file_lock
> *
> *  Semaphores
> *  ----------
> *  sesSem     operations on smb session
> *  tconSem    operations on tree connection
> *  fh_sem      file handle reconnection operations
>
>On Wed, Mar 25, 2020 at 6:30 PM Long Li <longli at microsoft.com> wrote:
>>
>> >Subject: Re: [PATCH] cifs: Remove locking in smb2_verify_signature()
>> >when calculating SMB2/SMB3 signature on receiving packets
>> >
>> >ср, 25 мар. 2020 г. в 11:15, <longli at linuxonhyperv.com>:
>> >>
>> >> From: Long Li <longli at microsoft.com>
>> >>
>> >> On the sending and receiving paths, CIFS uses the same cypto data
>> >> structures to calculate SMB2/SMB3 packet signatures. A lock on the
>> >> receiving path is necessary to control shared access to crypto data
>> >> structures. This lock degrades performance because it races with
>> >> the
>> >sending path.
>> >>
>> >> Define separate crypto data structures for sending and receiving
>> >> paths and remove this lock.
>> >>
>> >> Signed-off-by: Long Li <longli at microsoft.com>
>> >> ---
>> >>  fs/cifs/cifsencrypt.c   |  29 +++++----
>> >>  fs/cifs/cifsglob.h      |  21 +++++--
>> >>  fs/cifs/smb2proto.h     |   4 +-
>> >>  fs/cifs/smb2transport.c | 130
>> >> +++++++++++++++++++++++++---------------
>> >>  4 files changed, 119 insertions(+), 65 deletions(-)
>> >>
>> >> diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c index
>> >> 97b7497c13ef..222e8d13302c 100644
>> >> --- a/fs/cifs/cifsencrypt.c
>> >> +++ b/fs/cifs/cifsencrypt.c
>> >> @@ -804,16 +804,27 @@ calc_seckey(struct cifs_ses *ses)  void
>> >> cifs_crypto_secmech_release(struct TCP_Server_Info *server)  {
>> >> -       if (server->secmech.cmacaes) {
>> >> -               crypto_free_shash(server->secmech.cmacaes);
>> >> -               server->secmech.cmacaes = NULL;
>> >> -       }
>> >> +       int i;
>> >> +
>> >> +       for (i = CIFS_SECMECH_DIR_IN; i < CIFS_SECMECH_DIR_MAX; i++) {
>> >> +               if (server->secmech.cmacaes[i]) {
>> >> +                       crypto_free_shash(server->secmech.cmacaes[i]);
>> >> +                       server->secmech.cmacaes[i] = NULL;
>> >> +               }
>> >>
>> >> -       if (server->secmech.hmacsha256) {
>> >> -               crypto_free_shash(server->secmech.hmacsha256);
>> >> -               server->secmech.hmacsha256 = NULL;
>> >> +               if (server->secmech.hmacsha256[i]) {
>> >> +                       crypto_free_shash(server->secmech.hmacsha256[i]);
>> >> +                       server->secmech.hmacsha256[i] = NULL;
>> >> +               }
>> >> +
>> >> +               kfree(server->secmech.sdesccmacaes[i]);
>> >> +               server->secmech.sdesccmacaes[i] = NULL;
>> >> +
>> >> +               kfree(server->secmech.sdeschmacsha256[i]);
>> >> +               server->secmech.sdeschmacsha256[i] = NULL;
>> >>         }
>> >>
>> >> +
>> >>         if (server->secmech.md5) {
>> >>                 crypto_free_shash(server->secmech.md5);
>> >>                 server->secmech.md5 = NULL; @@ -839,10 +850,6 @@
>> >> cifs_crypto_secmech_release(struct TCP_Server_Info *server)
>> >>                 server->secmech.ccmaesdecrypt = NULL;
>> >>         }
>> >>
>> >> -       kfree(server->secmech.sdesccmacaes);
>> >> -       server->secmech.sdesccmacaes = NULL;
>> >> -       kfree(server->secmech.sdeschmacsha256);
>> >> -       server->secmech.sdeschmacsha256 = NULL;
>> >>         kfree(server->secmech.sdeschmacmd5);
>> >>         server->secmech.sdeschmacmd5 = NULL;
>> >>         kfree(server->secmech.sdescmd5); diff --git
>> >> a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h index
>> >> 0d956360e984..e31a902ebadc 100644
>> >> --- a/fs/cifs/cifsglob.h
>> >> +++ b/fs/cifs/cifsglob.h
>> >> @@ -137,17 +137,27 @@ struct sdesc {
>> >>         char ctx[];
>> >>  };
>> >>
>> >> +enum {
>> >> +       CIFS_SECMECH_DIR_IN = 0,
>> >> +       CIFS_SECMECH_DIR_OUT,
>> >> +       CIFS_SECMECH_DIR_MAX
>> >> +};
>> >> +
>> >>  /* crypto hashing related structure/fields, not specific to a sec
>> >> mech */  struct cifs_secmech {
>> >>         struct crypto_shash *hmacmd5; /* hmac-md5 hash function */
>> >>         struct crypto_shash *md5; /* md5 hash function */
>> >> -       struct crypto_shash *hmacsha256; /* hmac-sha256 hash function */
>> >> -       struct crypto_shash *cmacaes; /* block-cipher based MAC function
>*/
>> >> +       /* hmac-sha256 hash functions */
>> >> +       struct crypto_shash *hmacsha256[CIFS_SECMECH_DIR_MAX];
>> >> +       /* block-cipher based MAC function */
>> >> +       struct crypto_shash *cmacaes[CIFS_SECMECH_DIR_MAX];
>> >>         struct crypto_shash *sha512; /* sha512 hash function */
>> >>         struct sdesc *sdeschmacmd5;  /* ctxt to generate ntlmv2 hash, CR1
>*/
>> >>         struct sdesc *sdescmd5; /* ctxt to generate cifs/smb signature */
>> >> -       struct sdesc *sdeschmacsha256;  /* ctxt to generate smb2 signature
>*/
>> >> -       struct sdesc *sdesccmacaes;  /* ctxt to generate smb3 signature */
>> >> +       /* ctxt to generate smb2 signature */
>> >> +       struct sdesc *sdeschmacsha256[CIFS_SECMECH_DIR_MAX];
>> >> +       /* ctxt to generate smb3 signature */
>> >> +       struct sdesc *sdesccmacaes[CIFS_SECMECH_DIR_MAX];
>> >>         struct sdesc *sdescsha512; /* ctxt to generate smb3.11 signing key */
>> >>         struct crypto_aead *ccmaesencrypt; /* smb3 encryption aead */
>> >>         struct crypto_aead *ccmaesdecrypt; /* smb3 decryption aead
>> >> */ @@ -426,7 +436,8 @@ struct smb_version_operations {
>> >>         /* generate new lease key */
>> >>         void (*new_lease_key)(struct cifs_fid *);
>> >>         int (*generate_signingkey)(struct cifs_ses *);
>> >> -       int (*calc_signature)(struct smb_rqst *, struct TCP_Server_Info *);
>> >> +       int (*calc_signature)(struct smb_rqst *, struct TCP_Server_Info *,
>> >> +                             int direction);
>> >>         int (*set_integrity)(const unsigned int, struct cifs_tcon *tcon,
>> >>                              struct cifsFileInfo *src_file);
>> >>         int (*enum_snapshots)(const unsigned int xid, struct
>> >> cifs_tcon *tcon, diff --git a/fs/cifs/smb2proto.h
>> >> b/fs/cifs/smb2proto.h index
>> >> 4d1ff7b66fdc..f5edd6ea3639 100644
>> >> --- a/fs/cifs/smb2proto.h
>> >> +++ b/fs/cifs/smb2proto.h
>> >> @@ -55,9 +55,9 @@ extern struct cifs_ses *smb2_find_smb_ses(struct
>> >> TCP_Server_Info *server,  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,
>> >> -                               struct TCP_Server_Info *server);
>> >> +                               struct TCP_Server_Info *server, int
>> >> + direction);
>> >>  extern int smb3_calc_signature(struct smb_rqst *rqst,
>> >> -                               struct TCP_Server_Info *server);
>> >> +                               struct TCP_Server_Info *server, int
>> >> + direction);
>> >>  extern void smb2_echo_request(struct work_struct *work);  extern
>> >> __le32 smb2_get_lease_state(struct cifsInodeInfo *cinode);  extern
>> >> bool smb2_is_valid_oplock_break(char *buffer, diff --git
>> >> a/fs/cifs/smb2transport.c b/fs/cifs/smb2transport.c index
>> >> 08b703b7a15e..c8ba40d8fedf 100644
>> >> --- a/fs/cifs/smb2transport.c
>> >> +++ b/fs/cifs/smb2transport.c
>> >> @@ -43,30 +43,51 @@
>> >>  static int
>> >>  smb2_crypto_shash_allocate(struct TCP_Server_Info *server)  {
>> >> -       return cifs_alloc_hash("hmac(sha256)",
>> >> -                              &server->secmech.hmacsha256,
>> >> -                              &server->secmech.sdeschmacsha256);
>> >> +       int i, rc;
>> >> +
>> >> +       for (i = CIFS_SECMECH_DIR_IN; i < CIFS_SECMECH_DIR_MAX; i++) {
>> >> +               rc = cifs_alloc_hash("hmac(sha256)",
>> >> +                              &server->secmech.hmacsha256[i],
>> >> +                              &server->secmech.sdeschmacsha256[i]);
>> >> +               if (rc)
>> >> +                       goto fail;
>> >> +       }
>> >> +       return 0;
>> >> +
>> >> +fail:
>> >> +       for (i = CIFS_SECMECH_DIR_IN; i < CIFS_SECMECH_DIR_MAX; i++)
>> >> +               cifs_free_hash(
>> >> +                      &server->secmech.hmacsha256[i],
>> >> +                      &server->secmech.sdeschmacsha256[i]);
>> >> +       return rc;
>> >>  }
>> >>
>> >>  static int
>> >>  smb3_crypto_shash_allocate(struct TCP_Server_Info *server)  {
>> >>         struct cifs_secmech *p = &server->secmech;
>> >> -       int rc;
>> >> +       int i, rc;
>> >>
>> >> -       rc = cifs_alloc_hash("hmac(sha256)",
>> >> -                            &p->hmacsha256,
>> >> -                            &p->sdeschmacsha256);
>> >> -       if (rc)
>> >> -               goto err;
>> >> +       for (i = CIFS_SECMECH_DIR_IN; i < CIFS_SECMECH_DIR_MAX; i++) {
>> >> +               rc = cifs_alloc_hash("hmac(sha256)",
>> >> +                            &p->hmacsha256[i],
>> >> +                            &p->sdeschmacsha256[i]);
>> >> +               if (rc)
>> >> +                       goto fail;
>> >>
>> >> -       rc = cifs_alloc_hash("cmac(aes)", &p->cmacaes, &p->sdesccmacaes);
>> >> -       if (rc)
>> >> -               goto err;
>> >> +               rc = cifs_alloc_hash("cmac(aes)",
>> >> +                       &p->cmacaes[i], &p->sdesccmacaes[i]);
>> >>
>> >> +               if (rc)
>> >> +                       goto fail;
>> >> +       }
>> >>         return 0;
>> >> -err:
>> >> -       cifs_free_hash(&p->hmacsha256, &p->sdeschmacsha256);
>> >> +
>> >> +fail:
>> >> +       for (i = CIFS_SECMECH_DIR_IN; i < CIFS_SECMECH_DIR_MAX; i++) {
>> >> +               cifs_free_hash(&p->hmacsha256[i], &p->sdeschmacsha256[i]);
>> >> +               cifs_free_hash(&p->cmacaes[i], &p->sdesccmacaes[i]);
>> >> +       }
>> >>         return rc;
>> >>  }
>> >>
>> >> @@ -74,27 +95,32 @@ int
>> >>  smb311_crypto_shash_allocate(struct TCP_Server_Info *server)  {
>> >>         struct cifs_secmech *p = &server->secmech;
>> >> -       int rc = 0;
>> >> +       int i, rc = 0;
>> >>
>> >> -       rc = cifs_alloc_hash("hmac(sha256)",
>> >> -                            &p->hmacsha256,
>> >> -                            &p->sdeschmacsha256);
>> >> -       if (rc)
>> >> -               return rc;
>> >> +       for (i = CIFS_SECMECH_DIR_IN; i < CIFS_SECMECH_DIR_MAX; i++) {
>> >> +               rc = cifs_alloc_hash("hmac(sha256)",
>> >> +                            &p->hmacsha256[i],
>> >> +                            &p->sdeschmacsha256[i]);
>> >> +               if (rc)
>> >> +                       goto fail;
>> >>
>> >> -       rc = cifs_alloc_hash("cmac(aes)", &p->cmacaes, &p->sdesccmacaes);
>> >> -       if (rc)
>> >> -               goto err;
>> >> +               rc = cifs_alloc_hash("cmac(aes)",
>> >> +                       &p->cmacaes[i], &p->sdesccmacaes[i]);
>> >> +               if (rc)
>> >> +                       goto fail;
>> >> +       }
>> >>
>> >>         rc = cifs_alloc_hash("sha512", &p->sha512, &p->sdescsha512);
>> >>         if (rc)
>> >> -               goto err;
>> >> +               goto fail;
>> >>
>> >>         return 0;
>> >>
>> >> -err:
>> >> -       cifs_free_hash(&p->cmacaes, &p->sdesccmacaes);
>> >> -       cifs_free_hash(&p->hmacsha256, &p->sdeschmacsha256);
>> >> +fail:
>> >> +       for (i = CIFS_SECMECH_DIR_IN; i < CIFS_SECMECH_DIR_MAX; i++) {
>> >> +               cifs_free_hash(&p->hmacsha256[i], &p->sdeschmacsha256[i]);
>> >> +               cifs_free_hash(&p->cmacaes[i], &p->sdesccmacaes[i]);
>> >> +       }
>> >>         return rc;
>> >>  }
>> >>
>> >> @@ -219,7 +245,8 @@ smb2_find_smb_tcon(struct TCP_Server_Info
>> >*server, __u64 ses_id, __u32  tid)
>> >>  }
>> >>
>> >>  int
>> >> -smb2_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info
>> >*server)
>> >> +smb2_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info
>> >*server,
>> >> +                   int direction)
>> >>  {
>> >>         int rc;
>> >>         unsigned char smb2_signature[SMB2_HMACSHA256_SIZE];
>> >> @@ -229,6 +256,7 @@ smb2_calc_signature(struct smb_rqst *rqst,
>struct
>> >TCP_Server_Info *server)
>> >>         struct cifs_ses *ses;
>> >>         struct shash_desc *shash;
>> >>         struct smb_rqst drqst;
>> >> +       struct crypto_shash *hmacsha256;
>> >>
>> >>         ses = smb2_find_smb_ses(server, shdr->SessionId);
>> >>         if (!ses) {
>> >> @@ -245,14 +273,16 @@ smb2_calc_signature(struct smb_rqst *rqst,
>struct
>> >TCP_Server_Info *server)
>> >>                 return rc;
>> >>         }
>> >>
>> >> -       rc = crypto_shash_setkey(server->secmech.hmacsha256,
>> >> +       hmacsha256 = server->secmech.hmacsha256[direction];
>> >> +
>> >> +       rc = crypto_shash_setkey(hmacsha256,
>> >>                                  ses->auth_key.response,
>SMB2_NTLMV2_SESSKEY_SIZE);
>> >>         if (rc) {
>> >>                 cifs_server_dbg(VFS, "%s: Could not update with response\n",
>> >__func__);
>> >>                 return rc;
>> >>         }
>> >>
>> >> -       shash = &server->secmech.sdeschmacsha256->shash;
>> >> +       shash = &server->secmech.sdeschmacsha256[direction]->shash;
>> >>         rc = crypto_shash_init(shash);
>> >>         if (rc) {
>> >>                 cifs_server_dbg(VFS, "%s: Could not init sha256", __func__);
>> >> @@ -296,6 +326,8 @@ static int generate_key(struct cifs_ses *ses, struct
>> >kvec label,
>> >>         unsigned char prfhash[SMB2_HMACSHA256_SIZE];
>> >>         unsigned char *hashptr = prfhash;
>> >>         struct TCP_Server_Info *server = ses->server;
>> >> +       struct crypto_shash *hmacsha256;
>> >> +       struct sdesc *sdeschmacsha256;
>> >>
>> >>         memset(prfhash, 0x0, SMB2_HMACSHA256_SIZE);
>> >>         memset(key, 0x0, key_size);
>> >> @@ -306,55 +338,58 @@ static int generate_key(struct cifs_ses *ses,
>struct
>> >kvec label,
>> >>                 goto smb3signkey_ret;
>> >>         }
>> >>
>> >> -       rc = crypto_shash_setkey(server->secmech.hmacsha256,
>> >> +       hmacsha256 = server-
>> >>secmech.hmacsha256[CIFS_SECMECH_DIR_OUT];
>> >> +       sdeschmacsha256 = server-
>> >>secmech.sdeschmacsha256[CIFS_SECMECH_DIR_OUT];
>> >> +
>> >> +       rc = crypto_shash_setkey(hmacsha256,
>> >>                 ses->auth_key.response, SMB2_NTLMV2_SESSKEY_SIZE);
>> >>         if (rc) {
>> >>                 cifs_server_dbg(VFS, "%s: Could not set with session key\n",
>> >__func__);
>> >>                 goto smb3signkey_ret;
>> >>         }
>> >>
>> >> -       rc = crypto_shash_init(&server->secmech.sdeschmacsha256-
>>shash);
>> >> +       rc = crypto_shash_init(&sdeschmacsha256->shash);
>> >>         if (rc) {
>> >>                 cifs_server_dbg(VFS, "%s: Could not init sign hmac\n", __func__);
>> >>                 goto smb3signkey_ret;
>> >>         }
>> >>
>> >> -       rc = crypto_shash_update(&server->secmech.sdeschmacsha256-
>> >>shash,
>> >> +       rc = crypto_shash_update(&sdeschmacsha256->shash,
>> >>                                 i, 4);
>> >>         if (rc) {
>> >>                 cifs_server_dbg(VFS, "%s: Could not update with n\n",
>__func__);
>> >>                 goto smb3signkey_ret;
>> >>         }
>> >>
>> >> -       rc = crypto_shash_update(&server->secmech.sdeschmacsha256-
>> >>shash,
>> >> +       rc = crypto_shash_update(&sdeschmacsha256->shash,
>> >>                                 label.iov_base, label.iov_len);
>> >>         if (rc) {
>> >>                 cifs_server_dbg(VFS, "%s: Could not update with label\n",
>> >__func__);
>> >>                 goto smb3signkey_ret;
>> >>         }
>> >>
>> >> -       rc = crypto_shash_update(&server->secmech.sdeschmacsha256-
>> >>shash,
>> >> +       rc = crypto_shash_update(&sdeschmacsha256->shash,
>> >>                                 &zero, 1);
>> >>         if (rc) {
>> >>                 cifs_server_dbg(VFS, "%s: Could not update with zero\n",
>> >__func__);
>> >>                 goto smb3signkey_ret;
>> >>         }
>> >>
>> >> -       rc = crypto_shash_update(&server->secmech.sdeschmacsha256-
>> >>shash,
>> >> +       rc = crypto_shash_update(&sdeschmacsha256->shash,
>> >>                                 context.iov_base, context.iov_len);
>> >>         if (rc) {
>> >>                 cifs_server_dbg(VFS, "%s: Could not update with context\n",
>> >__func__);
>> >>                 goto smb3signkey_ret;
>> >>         }
>> >>
>> >> -       rc = crypto_shash_update(&server->secmech.sdeschmacsha256-
>> >>shash,
>> >> +       rc = crypto_shash_update(&sdeschmacsha256->shash,
>> >>                                 L, 4);
>> >>         if (rc) {
>> >>                 cifs_server_dbg(VFS, "%s: Could not update with L\n",
>__func__);
>> >>                 goto smb3signkey_ret;
>> >>         }
>> >>
>> >> -       rc = crypto_shash_final(&server->secmech.sdeschmacsha256-
>>shash,
>> >> +       rc = crypto_shash_final(&sdeschmacsha256->shash,
>> >>                                 hashptr);
>> >>         if (rc) {
>> >>                 cifs_server_dbg(VFS, "%s: Could not generate sha256 hash\n",
>> >__func__);
>> >> @@ -504,16 +539,18 @@ generate_smb311signingkey(struct cifs_ses
>*ses)
>> >>  }
>> >>
>> >>  int
>> >> -smb3_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info
>> >*server)
>> >> +smb3_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info
>> >*server,
>> >> +                   int direction)
>> >>  {
>> >>         int rc;
>> >>         unsigned char smb3_signature[SMB2_CMACAES_SIZE];
>> >>         unsigned char *sigptr = smb3_signature;
>> >>         struct kvec *iov = rqst->rq_iov;
>> >>         struct smb2_sync_hdr *shdr = (struct smb2_sync_hdr
>*)iov[0].iov_base;
>> >> -       struct shash_desc *shash = &server->secmech.sdesccmacaes-
>>shash;
>> >> +       struct shash_desc *shash;
>> >>         struct smb_rqst drqst;
>> >>         u8 key[SMB3_SIGN_KEY_SIZE];
>> >> +       struct crypto_shash *cmacaes;
>> >>
>> >>         rc = smb2_get_sign_key(shdr->SessionId, server, key);
>> >>         if (rc)
>> >> @@ -522,8 +559,10 @@ smb3_calc_signature(struct smb_rqst *rqst,
>struct
>> >TCP_Server_Info *server)
>> >>         memset(smb3_signature, 0x0, SMB2_CMACAES_SIZE);
>> >>         memset(shdr->Signature, 0x0, SMB2_SIGNATURE_SIZE);
>> >>
>> >> -       rc = crypto_shash_setkey(server->secmech.cmacaes,
>> >> -                                key, SMB2_CMACAES_SIZE);
>> >> +       cmacaes = server->secmech.cmacaes[direction];
>> >> +       shash = &server->secmech.sdesccmacaes[direction]->shash;
>> >> +
>> >> +       rc = crypto_shash_setkey(cmacaes, key, SMB2_CMACAES_SIZE);
>> >>         if (rc) {
>> >>                 cifs_server_dbg(VFS, "%s: Could not set key for cmac aes\n",
>> >__func__);
>> >>                 return rc;
>> >> @@ -593,8 +632,7 @@ smb2_sign_rqst(struct smb_rqst *rqst, struct
>> >TCP_Server_Info *server)
>> >>                 return 0;
>> >>         }
>> >>
>> >> -       rc = server->ops->calc_signature(rqst, server);
>> >> -
>> >> +       rc = server->ops->calc_signature(rqst, server,
>> >CIFS_SECMECH_DIR_OUT);
>> >>         return rc;
>> >>  }
>> >>
>> >> @@ -631,9 +669,7 @@ smb2_verify_signature(struct smb_rqst *rqst,
>struct
>> >TCP_Server_Info *server)
>> >>
>> >>         memset(shdr->Signature, 0, SMB2_SIGNATURE_SIZE);
>> >>
>> >> -       mutex_lock(&server->srv_mutex);
>> >> -       rc = server->ops->calc_signature(rqst, server);
>> >> -       mutex_unlock(&server->srv_mutex);
>> >> +       rc = server->ops->calc_signature(rqst, server,
>CIFS_SECMECH_DIR_IN);
>> >
>> >Multiple threads may be calling smb2_verify_signature from
>> >smb2_check_receive in separate threads (see compound_send_recv).
>What
>> >will prevent races on crypto data structures once the mutex around
>> >calc_signature is removed?
>>
>> It's my bad I will fix it. In this case we need another mutex for the receiving
>paths. I was looking at smb2_writev_callback() and smb2_readv_callback(),
>they are called from cifsd and also use this mutex. This mutex does the most
>damage in those two callback functions as it may block the receiving thread.
>>
>> Thanks,
>> Long
>>
>> >
>> >--
>> >Best regards,
>> >Pavel Shilovsky
>
>
>
>--
>Thanks,
>
>Steve


More information about the samba-technical mailing list