Re: [PATCH] extend IMA boot_aggregate with kernel measurements

From: kernel test robot <lkp@intel.com>
To: Maurizio Drocco <maurizio.drocco@ibm.com>,
	linux-integrity@vger.kernel.org
Cc: kbuild-all@lists.01.org, clang-built-linux@googlegroups.com,
	jejb@linux.ibm.com, Maurizio Drocco <maurizio.drocco@ibm.com>,
	Mimi Zohar <zohar@linux.ibm.com>,
	Dmitry Kasatkin <dmitry.kasatkin@gmail.com>,
	James Morris <jmorris@namei.org>,
	"Serge E. Hallyn" <serge@hallyn.com>,
	"open list:SECURITY SUBSYSTEM" 
	<linux-security-module@vger.kernel.org>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] extend IMA boot_aggregate with kernel measurements
Date: Fri, 12 Jun 2020 12:47:57 +0800	[thread overview]
Message-ID: <202006121210.R5q1RkS0%lkp@intel.com> (raw)
In-Reply-To: <20200611195422.2117-1-maurizio.drocco@ibm.com>

[-- Attachment #1: Type: text/plain, Size: 4066 bytes --]

Hi Maurizio,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on integrity/next-integrity]
[also build test WARNING on next-20200611]
[cannot apply to v5.7]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Maurizio-Drocco/extend-IMA-boot_aggregate-with-kernel-measurements/20200612-091504
base:   https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git next-integrity
config: x86_64-allyesconfig (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 3b43f006294971b8049d4807110032169780e5b8)
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
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>, old ones prefixed by <<):

>> security/integrity/ima/ima_crypto.c:838:35: warning: size argument in 'memcmp' call is a comparison [-Wmemsize-comparison]
crypto_shash_digestsize(tfm) != 0))
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
security/integrity/ima/ima_crypto.c:837:7: note: did you mean to compare the result of 'memcmp' instead?
if (memcmp(d.digest, d0.digest,
^
security/integrity/ima/ima_crypto.c:838:6: note: explicitly cast the argument to size_t to silence this warning
crypto_shash_digestsize(tfm) != 0))
^
(size_t)(                        )
1 warning generated.

vim +/memcmp +838 security/integrity/ima/ima_crypto.c

   797	
   798	/*
   799	 * The boot_aggregate is a cumulative hash over TPM registers 0 - 7.  With
   800	 * TPM 1.2 the boot_aggregate was based on reading the SHA1 PCRs, but with
   801	 * TPM 2.0 hash agility, TPM chips could support multiple TPM PCR banks,
   802	 * allowing firmware to configure and enable different banks.
   803	 *
   804	 * Knowing which TPM bank is read to calculate the boot_aggregate digest
   805	 * needs to be conveyed to a verifier.  For this reason, use the same
   806	 * hash algorithm for reading the TPM PCRs as for calculating the boot
   807	 * aggregate digest as stored in the measurement list.
   808	 */
   809	static int ima_calc_boot_aggregate_tfm(char *digest, u16 alg_id,
   810					       struct crypto_shash *tfm)
   811	{
   812		struct tpm_digest d = { .alg_id = alg_id, .digest = {0} }, d0 = d;
   813		int rc;
   814		u32 i;
   815		SHASH_DESC_ON_STACK(shash, tfm);
   816	
   817		shash->tfm = tfm;
   818	
   819		pr_devel("calculating the boot-aggregate based on TPM bank: %04x\n",
   820			 d.alg_id);
   821	
   822		rc = crypto_shash_init(shash);
   823		if (rc != 0)
   824			return rc;
   825	
   826		/* cumulative sha1 over tpm registers 0-7 */
   827		for (i = TPM_PCR0; i < TPM_PCR8; i++) {
   828			ima_pcrread(i, &d);
   829			/* now accumulate with current aggregate */
   830			rc = crypto_shash_update(shash, d.digest,
   831						 crypto_shash_digestsize(tfm));
   832		}
   833		/* extend cumulative sha1 over tpm registers 8-9 */
   834		for (i = TPM_PCR8; i < TPM_PCR10; i++) {
   835			ima_pcrread(i, &d);
   836			/* if not zero, accumulate with current aggregate */
   837			if (memcmp(d.digest, d0.digest,
 > 838						crypto_shash_digestsize(tfm) != 0))
   839				rc = crypto_shash_update(shash, d.digest,
   840						crypto_shash_digestsize(tfm));
   841		}
   842		if (!rc)
   843			crypto_shash_final(shash, digest);
   844		return rc;
   845	}
   846	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 73441 bytes --]