| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 3 Apr 2020 10:45:35 -0700
From: Pavel Shilovsky <piastryyy@...il.com>
To: Long Li <longli@...rosoft.com>
Cc: Steve French <sfrench@...ba.org>,
linux-cifs <linux-cifs@...r.kernel.org>,
samba-technical <samba-technical@...ts.samba.org>,
Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] cifs: Allocate crypto structures on the fly for
calculating signatures of incoming packets
вт, 31 мар. 2020 г. в 16:22, <longli@...uxonhyperv.com>:
>
> From: Long Li <longli@...rosoft.com>
>
> 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@...rosoft.com>
> ---
> 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.
Now the code doesn't allocate those variables at all. Unlike SMB3
where structures are allocated in during key generation, for SMB2 we
do it on demand in smb2_calc_signature(). So, the code above should be
changed to call smb2_crypto_shash_allocate() in "else" block.
--
Best regards,
Pavel Shilovsky
Powered by blists - more mailing lists