From: kernel test robot <lkp@intel.com>
To: James Bottomley <James.Bottomley@hansenpartnership.com>,
linux-integrity@vger.kernel.org
Cc: oe-kbuild-all@lists.linux.dev,
Jarkko Sakkinen <jarkko@kernel.org>,
keyrings@vger.kernel.org, Ard Biesheuvel <ardb@kernel.org>
Subject: Re: [PATCH v2 06/11] tpm: Add full HMAC and encrypt/decrypt session handling code
Date: Wed, 25 Jan 2023 07:11:58 +0800 [thread overview]
Message-ID: <202301250706.deGvd0yq-lkp@intel.com> (raw)
In-Reply-To: <20230124175516.5984-7-James.Bottomley@HansenPartnership.com>
Hi James,
I love your patch! Perhaps something to improve:
[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on char-misc/char-misc-next char-misc/char-misc-linus zohar-integrity/next-integrity linus/master v6.2-rc5 next-20230124]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/James-Bottomley/tpm-move-buffer-handling-from-static-inlines-to-real-functions/20230125-020146
patch link: https://lore.kernel.org/r/20230124175516.5984-7-James.Bottomley%40HansenPartnership.com
patch subject: [PATCH v2 06/11] tpm: Add full HMAC and encrypt/decrypt session handling code
config: arc-allyesconfig (https://download.01.org/0day-ci/archive/20230125/202301250706.deGvd0yq-lkp@intel.com/config)
compiler: arceb-elf-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/dc0fc74718b4a786aba4a954233e8ab3afdcc03c
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review James-Bottomley/tpm-move-buffer-handling-from-static-inlines-to-real-functions/20230125-020146
git checkout dc0fc74718b4a786aba4a954233e8ab3afdcc03c
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arc olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash drivers/char/tpm/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
drivers/char/tpm/tpm2-sessions.c:1184:5: warning: no previous prototype for 'tpm2_create_null_primary' [-Wmissing-prototypes]
1184 | int tpm2_create_null_primary(struct tpm_chip *chip) {
| ^~~~~~~~~~~~~~~~~~~~~~~~
drivers/char/tpm/tpm2-sessions.c: In function 'tpm_buf_check_hmac_response':
>> drivers/char/tpm/tpm2-sessions.c:831:1: warning: the frame size of 1132 bytes is larger than 1024 bytes [-Wframe-larger-than=]
831 | }
| ^
drivers/char/tpm/tpm2-sessions.c: In function 'tpm_buf_fill_hmac_session':
drivers/char/tpm/tpm2-sessions.c:579:1: warning: the frame size of 1132 bytes is larger than 1024 bytes [-Wframe-larger-than=]
579 | }
| ^
vim +831 drivers/char/tpm/tpm2-sessions.c
676
677 /**
678 * tpm_buf_check_hmac_response() - check the TPM return HMAC for correctness
679 * @buf: the original command buffer (which now contains the response)
680 * @auth: the auth structure allocated by tpm2_start_auth_session()
681 * @rc: the return code from tpm_transmit_cmd
682 *
683 * If @rc is non zero, @buf may not contain an actual return, so @rc
684 * is passed through as the return and the session cleaned up and
685 * de-allocated if required (this is required if
686 * TPM2_SA_CONTINUE_SESSION was not specified as a session flag).
687 *
688 * If @rc is zero, the response HMAC is computed against the returned
689 * @buf and matched to the TPM one in the session area. If there is a
690 * mismatch, an error is logged and -EINVAL returned.
691 *
692 * The reason for this is that the command issue and HMAC check
693 * sequence should look like:
694 *
695 * rc = tpm_transmit_cmd(...);
696 * rc = tpm_buf_check_hmac_response(&buf, auth, rc);
697 * if (rc)
698 * ...
699 *
700 * Which is easily layered into the current contrl flow.
701 *
702 * Returns: 0 on success or an error.
703 */
704 int tpm_buf_check_hmac_response(struct tpm_buf *buf, struct tpm2_auth *auth,
705 int rc)
706 {
707 struct tpm_header *head = (struct tpm_header *)buf->data;
708 struct tpm_chip *chip = auth->chip;
709 const u8 *s, *p;
710 u8 rphash[SHA256_DIGEST_SIZE];
711 u32 attrs;
712 SHASH_DESC_ON_STACK(desc, sha256_hash);
713 u16 tag = be16_to_cpu(head->tag);
714 u32 cc = be32_to_cpu(auth->ordinal);
715 int parm_len, len, i, handles;
716
717 if (auth->session >= TPM_HEADER_SIZE) {
718 WARN(1, "tpm session not filled correctly\n");
719 goto out;
720 }
721
722 if (rc != 0)
723 /* pass non success rc through and close the session */
724 goto out;
725
726 rc = -EINVAL;
727 if (tag != TPM2_ST_SESSIONS) {
728 dev_err(&chip->dev, "TPM: HMAC response check has no sessions tag\n");
729 goto out;
730 }
731
732 i = tpm2_find_cc(chip, cc);
733 if (i < 0)
734 goto out;
735 attrs = chip->cc_attrs_tbl[i];
736 handles = (attrs >> TPM2_CC_ATTR_RHANDLE) & 1;
737
738 /* point to area beyond handles */
739 s = &buf->data[TPM_HEADER_SIZE + handles * 4];
740 parm_len = tpm_get_inc_u32(&s);
741 p = s;
742 s += parm_len;
743 /* skip over any sessions before ours */
744 for (i = 0; i < auth->session - 1; i++) {
745 len = tpm_get_inc_u16(&s);
746 s += len + 1;
747 len = tpm_get_inc_u16(&s);
748 s += len;
749 }
750 /* TPM nonce */
751 len = tpm_get_inc_u16(&s);
752 if (s - buf->data + len > tpm_buf_length(buf))
753 goto out;
754 if (len != SHA256_DIGEST_SIZE)
755 goto out;
756 memcpy(auth->tpm_nonce, s, len);
757 s += len;
758 attrs = *s++;
759 len = tpm_get_inc_u16(&s);
760 if (s - buf->data + len != tpm_buf_length(buf))
761 goto out;
762 if (len != SHA256_DIGEST_SIZE)
763 goto out;
764 /*
765 * s points to the HMAC. now calculate comparison, beginning
766 * with rphash
767 */
768 desc->tfm = sha256_hash;
769 crypto_shash_init(desc);
770 /* yes, I know this is now zero, but it's what the standard says */
771 crypto_shash_update(desc, (u8 *)&head->return_code,
772 sizeof(head->return_code));
773 /* ordinal is already BE */
774 crypto_shash_update(desc, (u8 *)&auth->ordinal, sizeof(auth->ordinal));
775 crypto_shash_update(desc, p, parm_len);
776 crypto_shash_final(desc, rphash);
777
778 /* now calculate the hmac */
779 hmac_init(desc, auth->session_key, sizeof(auth->session_key)
780 + auth->passphraselen);
781 crypto_shash_update(desc, rphash, sizeof(rphash));
782 crypto_shash_update(desc, auth->tpm_nonce, sizeof(auth->tpm_nonce));
783 crypto_shash_update(desc, auth->our_nonce, sizeof(auth->our_nonce));
784 crypto_shash_update(desc, &auth->attrs, 1);
785 /* we're done with the rphash, so put our idea of the hmac there */
786 hmac_final(desc, auth->session_key, sizeof(auth->session_key)
787 + auth->passphraselen, rphash);
788 if (memcmp(rphash, s, SHA256_DIGEST_SIZE) == 0) {
789 rc = 0;
790 } else {
791 dev_err(&auth->chip->dev, "TPM: HMAC check failed\n");
792 goto out;
793 }
794
795 /* now do response decryption */
796 if (auth->attrs & TPM2_SA_ENCRYPT) {
797 struct scatterlist sg[1];
798 SYNC_SKCIPHER_REQUEST_ON_STACK(req, auth->aes);
799 DECLARE_CRYPTO_WAIT(wait);
800
801 skcipher_request_set_sync_tfm(req, auth->aes);
802 skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_SLEEP,
803 crypto_req_done, &wait);
804
805 /* need key and IV */
806 KDFa(auth->session_key, SHA256_DIGEST_SIZE
807 + auth->passphraselen, "CFB", auth->tpm_nonce,
808 auth->our_nonce, AES_KEYBYTES + AES_BLOCK_SIZE,
809 auth->scratch);
810 crypto_sync_skcipher_setkey(auth->aes, auth->scratch, AES_KEYBYTES);
811 len = tpm_get_inc_u16(&p);
812 sg_init_one(sg, p, len);
813 skcipher_request_set_crypt(req, sg, sg, len,
814 auth->scratch + AES_KEYBYTES);
815 crypto_wait_req(crypto_skcipher_decrypt(req), &wait);
816 }
817
818 out:
819 if ((auth->attrs & TPM2_SA_CONTINUE_SESSION) == 0) {
820 /* manually close the session if it wasn't consumed */
821 if (rc)
822 tpm2_flush_context(chip, auth->handle);
823 crypto_free_sync_skcipher(auth->aes);
824 kfree(auth);
825 } else {
826 /* reset for next use */
827 auth->session = TPM_HEADER_SIZE;
828 }
829
830 return rc;
> 831 }
832 EXPORT_SYMBOL(tpm_buf_check_hmac_response);
833
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
next prev parent reply other threads:[~2023-01-24 23:12 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-24 17:55 [PATCH v2 00/11] add integrity and security to TPM2 transactions James Bottomley
2023-01-24 17:55 ` [PATCH v2 01/11] tpm: move buffer handling from static inlines to real functions James Bottomley
2023-01-24 19:57 ` kernel test robot
2023-01-25 14:01 ` James Bottomley
2023-01-24 17:55 ` [PATCH v2 02/11] tpm: add buffer handling for TPM2B types James Bottomley
2023-01-24 17:55 ` [PATCH v2 03/11] tpm: add cursor based buffer functions for response parsing James Bottomley
2023-01-24 17:55 ` [PATCH v2 04/11] tpm: add buffer function to point to returned parameters James Bottomley
2023-01-24 17:55 ` [PATCH v2 05/11] tpm: export the context save and load commands James Bottomley
2023-01-24 17:55 ` [PATCH v2 06/11] tpm: Add full HMAC and encrypt/decrypt session handling code James Bottomley
2023-01-24 20:48 ` kernel test robot
2023-01-24 23:11 ` kernel test robot [this message]
2023-01-25 12:59 ` James Bottomley
2023-02-03 6:06 ` Yujie Liu
2023-02-08 2:49 ` Jarkko Sakkinen
2023-02-10 14:48 ` James Bottomley
2023-02-13 7:45 ` Jarkko Sakkinen
2023-02-13 9:31 ` Yujie Liu
2023-02-14 13:54 ` Ard Biesheuvel
2023-02-14 14:28 ` James Bottomley
2023-02-14 14:36 ` Ard Biesheuvel
2023-02-16 14:52 ` James Bottomley
2023-02-17 8:49 ` Ard Biesheuvel
2023-02-14 14:34 ` James Bottomley
2023-02-17 21:51 ` Jarkko Sakkinen
2023-02-08 4:35 ` James Bottomley
2023-01-25 6:03 ` kernel test robot
2023-01-24 17:55 ` [PATCH v2 07/11] tpm: add hmac checks to tpm2_pcr_extend() James Bottomley
2023-01-24 17:55 ` [PATCH v2 08/11] tpm: add session encryption protection to tpm2_get_random() James Bottomley
2023-01-24 17:55 ` [PATCH v2 09/11] KEYS: trusted: Add session encryption protection to the seal/unseal path James Bottomley
2023-01-29 13:06 ` Ben Boeckel
2023-01-24 17:55 ` [PATCH v2 10/11] tpm: add the null key name as a sysfs export James Bottomley
2023-01-24 17:55 ` [PATCH v2 11/11] Documentation: add tpm-security.rst James Bottomley
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=202301250706.deGvd0yq-lkp@intel.com \
--to=lkp@intel.com \
--cc=James.Bottomley@hansenpartnership.com \
--cc=ardb@kernel.org \
--cc=jarkko@kernel.org \
--cc=keyrings@vger.kernel.org \
--cc=linux-integrity@vger.kernel.org \
--cc=oe-kbuild-all@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).