Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933375AbcDSQMr (ORCPT ); Tue, 19 Apr 2016 12:12:47 -0400 Received: from mail-pf0-f179.google.com ([209.85.192.179]:34462 "EHLO mail-pf0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933361AbcDSQMo (ORCPT ); Tue, 19 Apr 2016 12:12:44 -0400 Message-ID: <1461082354.15135.6.camel@poochiereds.net> Subject: Re: [PATCH 2/6] cifs: merge the hash calculation helpers From: Jeff Layton To: Al Viro , linux-cifs@vger.kernel.org Cc: linux-kernel@vger.kernel.org Date: Tue, 19 Apr 2016 12:12:34 -0400 In-Reply-To: <20160409205056.GH25498@ZenIV.linux.org.uk> References: <20160409204301.GF25498@ZenIV.linux.org.uk> <20160409205056.GH25498@ZenIV.linux.org.uk> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.18.5.2 (3.18.5.2-1.fc23) Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 10452 Lines: 313 On Sat, 2016-04-09 at 21:50 +0100, Al Viro wrote: > three practically identical copies... > > Signed-off-by: Al Viro > --- >  fs/cifs/cifsencrypt.c   |  97 ++++++++++++++++++++++++------------------- >  fs/cifs/cifsproto.h     |   3 ++ >  fs/cifs/smb2transport.c | 107 +++++------------------------------------------- >  3 files changed, 67 insertions(+), 140 deletions(-) > > diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c > index 4897dac..6aeb8d4 100644 > --- a/fs/cifs/cifsencrypt.c > +++ b/fs/cifs/cifsencrypt.c > @@ -66,45 +66,15 @@ cifs_crypto_shash_md5_allocate(struct TCP_Server_Info *server) >   return 0; >  } >   > -/* > - * Calculate and return the CIFS signature based on the mac key and SMB PDU. > - * The 16 byte signature must be allocated by the caller. Note we only use the > - * 1st eight bytes and that the smb header signature field on input contains > - * the sequence number before this function is called. Also, this function > - * should be called with the server->srv_mutex held. > - */ > -static int cifs_calc_signature(struct smb_rqst *rqst, > - struct TCP_Server_Info *server, char *signature) > +int __cifs_calc_signature(struct smb_rqst *rqst, > + struct TCP_Server_Info *server, char *signature, > + struct shash_desc *shash) >  { >   int i; >   int rc; >   struct kvec *iov = rqst->rq_iov; >   int n_vec = rqst->rq_nvec; >   > - if (iov == NULL || signature == NULL || server == NULL) > - return -EINVAL; > - > - if (!server->secmech.sdescmd5) { > - rc = cifs_crypto_shash_md5_allocate(server); > - if (rc) { > - cifs_dbg(VFS, "%s: Can't alloc md5 crypto\n", __func__); > - return -1; > - } > - } > - > - rc = crypto_shash_init(&server->secmech.sdescmd5->shash); > - if (rc) { > - cifs_dbg(VFS, "%s: Could not init md5\n", __func__); > - return rc; > - } > - > - rc = crypto_shash_update(&server->secmech.sdescmd5->shash, > - server->session_key.response, server->session_key.len); > - if (rc) { > - cifs_dbg(VFS, "%s: Could not update with response\n", __func__); > - return rc; > - } > - >   for (i = 0; i < n_vec; i++) { >   if (iov[i].iov_len == 0) >   continue; > @@ -117,12 +87,10 @@ static int cifs_calc_signature(struct smb_rqst *rqst, >   if (i == 0) { >   if (iov[0].iov_len <= 8) /* cmd field at offset 9 */ >   break; /* nothing to sign or corrupt header */ > - rc = > - crypto_shash_update(&server->secmech.sdescmd5->shash, > + rc = crypto_shash_update(shash, >   iov[i].iov_base + 4, iov[i].iov_len - 4); >   } else { > - rc = > - crypto_shash_update(&server->secmech.sdescmd5->shash, > + rc = crypto_shash_update(shash, >   iov[i].iov_base, iov[i].iov_len); >   } >   if (rc) { > @@ -134,21 +102,64 @@ static int cifs_calc_signature(struct smb_rqst *rqst, >   >   /* now hash over the rq_pages array */ >   for (i = 0; i < rqst->rq_npages; i++) { > - struct kvec p_iov; > + void *kaddr = kmap(rqst->rq_pages[i]); > + size_t len = rqst->rq_pagesz; > + > + if (i == rqst->rq_npages - 1) > + len = rqst->rq_tailsz; > + > + crypto_shash_update(shash, kaddr, len); >   > - cifs_rqst_page_to_kvec(rqst, i, &p_iov); > - crypto_shash_update(&server->secmech.sdescmd5->shash, > - p_iov.iov_base, p_iov.iov_len); >   kunmap(rqst->rq_pages[i]); >   } >   > - rc = crypto_shash_final(&server->secmech.sdescmd5->shash, signature); > + rc = crypto_shash_final(shash, signature); >   if (rc) > - cifs_dbg(VFS, "%s: Could not generate md5 hash\n", __func__); > + cifs_dbg(VFS, "%s: Could not generate hash\n", __func__); >   >   return rc; >  } >   > +/* > + * Calculate and return the CIFS signature based on the mac key and SMB PDU. > + * The 16 byte signature must be allocated by the caller. Note we only use the > + * 1st eight bytes and that the smb header signature field on input contains > + * the sequence number before this function is called. Also, this function > + * should be called with the server->srv_mutex held. > + */ > +static int cifs_calc_signature(struct smb_rqst *rqst, > + struct TCP_Server_Info *server, char *signature) > +{ > + int rc; > + > + if (!rqst->rq_iov || !signature || !server) > + return -EINVAL; > + > + if (!server->secmech.sdescmd5) { > + rc = cifs_crypto_shash_md5_allocate(server); > + if (rc) { > + cifs_dbg(VFS, "%s: Can't alloc md5 crypto\n", __func__); > + return -1; > + } > + } > + > + rc = crypto_shash_init(&server->secmech.sdescmd5->shash); > + if (rc) { > + cifs_dbg(VFS, "%s: Could not init md5\n", __func__); > + return rc; > + } > + > + rc = crypto_shash_update(&server->secmech.sdescmd5->shash, > + server->session_key.response, server->session_key.len); > + if (rc) { > + cifs_dbg(VFS, "%s: Could not update with response\n", __func__); > + return rc; > + } > + > + return __cifs_calc_signature(rqst, server, signature, > +      &server->secmech.sdescmd5->shash); > +} > + >  /* must be called with server->srv_mutex held */ >  int cifs_sign_rqst(struct smb_rqst *rqst, struct TCP_Server_Info *server, >      __u32 *pexpected_response_sequence_number) > diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h > index eed7ff5..d9b4f44 100644 > --- a/fs/cifs/cifsproto.h > +++ b/fs/cifs/cifsproto.h > @@ -512,4 +512,7 @@ int cifs_create_mf_symlink(unsigned int xid, struct cifs_tcon *tcon, >      struct cifs_sb_info *cifs_sb, >      const unsigned char *path, char *pbuf, >      unsigned int *pbytes_written); > +int __cifs_calc_signature(struct smb_rqst *rqst, > + struct TCP_Server_Info *server, char *signature, > + struct shash_desc *shash); >  #endif /* _CIFSPROTO_H */ > diff --git a/fs/cifs/smb2transport.c b/fs/cifs/smb2transport.c > index 8732a43..bc9a7b6 100644 > --- a/fs/cifs/smb2transport.c > +++ b/fs/cifs/smb2transport.c > @@ -135,11 +135,10 @@ smb2_find_smb_ses(struct smb2_hdr *smb2hdr, struct TCP_Server_Info *server) >  int >  smb2_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server) >  { > - int i, rc; > + int rc; >   unsigned char smb2_signature[SMB2_HMACSHA256_SIZE]; >   unsigned char *sigptr = smb2_signature; >   struct kvec *iov = rqst->rq_iov; > - int n_vec = rqst->rq_nvec; >   struct smb2_hdr *smb2_pdu = (struct smb2_hdr *)iov[0].iov_base; >   struct cifs_ses *ses; >   > @@ -171,53 +170,11 @@ smb2_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server) >   return rc; >   } >   > - for (i = 0; i < n_vec; i++) { > - if (iov[i].iov_len == 0) > - continue; > - if (iov[i].iov_base == NULL) { > - cifs_dbg(VFS, "null iovec entry\n"); > - return -EIO; > - } > - /* > -  * The first entry includes a length field (which does not get > -  * signed that occupies the first 4 bytes before the header). > -  */ > - if (i == 0) { > - if (iov[0].iov_len <= 8) /* cmd field at offset 9 */ > - break; /* nothing to sign or corrupt header */ > - rc = > - crypto_shash_update( > - &server->secmech.sdeschmacsha256->shash, > - iov[i].iov_base + 4, iov[i].iov_len - 4); > - } else { > - rc = > - crypto_shash_update( > - &server->secmech.sdeschmacsha256->shash, > - iov[i].iov_base, iov[i].iov_len); > - } > - if (rc) { > - cifs_dbg(VFS, "%s: Could not update with payload\n", > -  __func__); > - return rc; > - } > - } > - > - /* now hash over the rq_pages array */ > - for (i = 0; i < rqst->rq_npages; i++) { > - struct kvec p_iov; > - > - cifs_rqst_page_to_kvec(rqst, i, &p_iov); > - crypto_shash_update(&server->secmech.sdeschmacsha256->shash, > - p_iov.iov_base, p_iov.iov_len); > - kunmap(rqst->rq_pages[i]); > - } > - > - rc = crypto_shash_final(&server->secmech.sdeschmacsha256->shash, > - sigptr); > - if (rc) > - cifs_dbg(VFS, "%s: Could not generate sha256 hash\n", __func__); > + rc = __cifs_calc_signature(rqst, server, sigptr, > + &server->secmech.sdeschmacsha256->shash); >   > - memcpy(smb2_pdu->Signature, sigptr, SMB2_SIGNATURE_SIZE); > + if (!rc) > + memcpy(smb2_pdu->Signature, sigptr, SMB2_SIGNATURE_SIZE); >   >   return rc; >  } > @@ -395,12 +352,10 @@ generate_smb311signingkey(struct cifs_ses *ses) >  int >  smb3_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server) >  { > - int i; >   int rc = 0; >   unsigned char smb3_signature[SMB2_CMACAES_SIZE]; >   unsigned char *sigptr = smb3_signature; >   struct kvec *iov = rqst->rq_iov; > - int n_vec = rqst->rq_nvec; >   struct smb2_hdr *smb2_pdu = (struct smb2_hdr *)iov[0].iov_base; >   struct cifs_ses *ses; >   > @@ -431,54 +386,12 @@ smb3_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server) >   cifs_dbg(VFS, "%s: Could not init cmac aes\n", __func__); >   return rc; >   } > + > + rc = __cifs_calc_signature(rqst, server, sigptr, > +    &server->secmech.sdesccmacaes->shash); >   > - for (i = 0; i < n_vec; i++) { > - if (iov[i].iov_len == 0) > - continue; > - if (iov[i].iov_base == NULL) { > - cifs_dbg(VFS, "null iovec entry"); > - return -EIO; > - } > - /* > -  * The first entry includes a length field (which does not get > -  * signed that occupies the first 4 bytes before the header). > -  */ > - if (i == 0) { > - if (iov[0].iov_len <= 8) /* cmd field at offset 9 */ > - break; /* nothing to sign or corrupt header */ > - rc = > - crypto_shash_update( > - &server->secmech.sdesccmacaes->shash, > - iov[i].iov_base + 4, iov[i].iov_len - 4); > - } else { > - rc = > - crypto_shash_update( > - &server->secmech.sdesccmacaes->shash, > - iov[i].iov_base, iov[i].iov_len); > - } > - if (rc) { > - cifs_dbg(VFS, "%s: Couldn't update cmac aes with payload\n", > - __func__); > - return rc; > - } > - } > - > - /* now hash over the rq_pages array */ > - for (i = 0; i < rqst->rq_npages; i++) { > - struct kvec p_iov; > - > - cifs_rqst_page_to_kvec(rqst, i, &p_iov); > - crypto_shash_update(&server->secmech.sdesccmacaes->shash, > - p_iov.iov_base, p_iov.iov_len); > - kunmap(rqst->rq_pages[i]); > - } > - > - rc = crypto_shash_final(&server->secmech.sdesccmacaes->shash, > - sigptr); > - if (rc) > - cifs_dbg(VFS, "%s: Could not generate cmac aes\n", __func__); > - > - memcpy(smb2_pdu->Signature, sigptr, SMB2_SIGNATURE_SIZE); > + if (!rc) > + memcpy(smb2_pdu->Signature, sigptr, SMB2_SIGNATURE_SIZE); >   >   return rc; >  } Nice. Long overdue cleanup. Reviewed-by: Jeff Layton