Re: [PATCH] cifs: Allocate crypto structures on the fly for calculating signatures of incoming packets

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



пт, 3 апр. 2020 г. в 16:11, Long Li <longli@xxxxxxxxxxxxx>:
>
> >Subject: Re: [PATCH] cifs: Allocate crypto structures on the fly for calculating
> >signatures of incoming packets
> >
> >вт, 31 мар. 2020 г. в 16:22, <longli@xxxxxxxxxxxxxxxxx>:
> >>
> >> From: Long Li <longli@xxxxxxxxxxxxx>
> >>
> >> CIFS uses pre-allocated crypto structures to calculate signatures for
> >> both incoming and outgoing packets. In this way it doesn't need to
> >> allocate crypto structures for every packet, but it requires a lock to
> >> prevent concurrent access to crypto structures.
> >>
> >> Remove the lock by allocating crypto structures on the fly for
> >> incoming packets. At the same time, we can still use pre-allocated
> >> crypto structures for outgoing packets, as they are already protected
> >> by transport lock srv_mutex.
> >>
> >> Signed-off-by: Long Li <longli@xxxxxxxxxxxxx>
> >> ---
> >>  fs/cifs/cifsglob.h      |  3 +-
> >>  fs/cifs/smb2proto.h     |  6 ++-
> >>  fs/cifs/smb2transport.c | 87
> >> +++++++++++++++++++++++++----------------
> >>  3 files changed, 60 insertions(+), 36 deletions(-)
> >>
> >> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h index
> >> 0d956360e984..7448e7202e7a 100644
> >> --- a/fs/cifs/cifsglob.h
> >> +++ b/fs/cifs/cifsglob.h
> >> @@ -426,7 +426,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 *,
> >> +                               bool allocate_crypto);
> >>         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..087d5f14320b 100644
> >> --- a/fs/cifs/smb2proto.h
> >> +++ b/fs/cifs/smb2proto.h
> >> @@ -55,9 +55,11 @@ 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,
> >> +                               bool allocate_crypto);
> >>  extern int smb3_calc_signature(struct smb_rqst *rqst,
> >> -                               struct TCP_Server_Info *server);
> >> +                               struct TCP_Server_Info *server,
> >> +                               bool allocate_crypto);
> >>  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..c01e19a3b112 100644
> >> --- a/fs/cifs/smb2transport.c
> >> +++ b/fs/cifs/smb2transport.c
> >> @@ -40,14 +40,6 @@
> >>  #include "smb2status.h"
> >>  #include "smb2glob.h"
> >>
> >> -static int
> >> -smb2_crypto_shash_allocate(struct TCP_Server_Info *server) -{
> >> -       return cifs_alloc_hash("hmac(sha256)",
> >> -                              &server->secmech.hmacsha256,
> >> -                              &server->secmech.sdeschmacsha256);
> >> -}
> >> -
> >>  static int
> >>  smb3_crypto_shash_allocate(struct TCP_Server_Info *server)  { @@
> >> -219,7 +211,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,
> >> +                       bool allocate_crypto)
> >>  {
> >>         int rc;
> >>         unsigned char smb2_signature[SMB2_HMACSHA256_SIZE];
> >> @@ -228,6 +221,8 @@ smb2_calc_signature(struct smb_rqst *rqst, struct
> >TCP_Server_Info *server)
> >>         struct smb2_sync_hdr *shdr = (struct smb2_sync_hdr *)iov[0].iov_base;
> >>         struct cifs_ses *ses;
> >>         struct shash_desc *shash;
> >> +       struct crypto_shash *hash;
> >> +       struct sdesc *sdesc = NULL;
> >>         struct smb_rqst drqst;
> >>
> >>         ses = smb2_find_smb_ses(server, shdr->SessionId); @@ -239,24
> >> +234,32 @@ smb2_calc_signature(struct smb_rqst *rqst, struct
> >TCP_Server_Info *server)
> >>         memset(smb2_signature, 0x0, SMB2_HMACSHA256_SIZE);
> >>         memset(shdr->Signature, 0x0, SMB2_SIGNATURE_SIZE);
> >>
> >> -       rc = smb2_crypto_shash_allocate(server);
> >> -       if (rc) {
> >> -               cifs_server_dbg(VFS, "%s: sha256 alloc failed\n", __func__);
> >> -               return rc;
> >> +       if (allocate_crypto) {
> >> +               rc = cifs_alloc_hash("hmac(sha256)", &hash, &sdesc);
> >> +               if (rc) {
> >> +                       cifs_server_dbg(VFS,
> >> +                                       "%s: sha256 alloc failed\n", __func__);
> >> +                       return rc;
> >> +               }
> >> +               shash = &sdesc->shash;
> >> +       } else {
> >> +               hash = server->secmech.hmacsha256;
> >> +               shash = &server->secmech.sdeschmacsha256->shash;
> >>         }
> >
> >smb2_crypto_shash_allocate() unconditionally allocated
> >server->secmech.hmacsha256 and server->secmech.sdeschmacsha256-
> >>shash.
>
> I think they are allocated in smb311_crypto_shash_allocate(), through
> => smb311_crypto_shash_allocate
>  => smb311_update_preauth_hash
>  => compound_send_recv
>  => cifs_send_recv
>  => SMB2_negotiate
>
> The function names are a little misleading...

smb311_crypto_shash_allocate() only allocate those structures for SMB
3.1.1 protocol version, see the code below:

797 int
798 smb311_update_preauth_hash(struct cifs_ses *ses, struct kvec *iov, int nvec)
799 {
800 >-------int i, rc;
801 >-------struct sdesc *d;
802 >-------struct smb2_sync_hdr *hdr;
803
804 >-------if (ses->server->tcpStatus == CifsGood) {
805 >------->-------/* skip non smb311 connections */
806 >------->-------if (ses->server->dialect != SMB311_PROT_ID)
807 >------->------->-------return 0;

--
Best regards,
Pavel Shilovsky




[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux