Re: [PATCH v2 06/11] tpm: Add full HMAC and encrypt/decrypt session handling code

[kernel test robot <lkp@intel.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