Message ID | 20211129170057.243127-2-zohar@linux.ibm.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | ima: support fs-verity signatures stored as | expand |
Hi Mimi,
I love your patch! Yet something to improve:
[auto build test ERROR on zohar-integrity/next-integrity]
[also build test ERROR on fscrypt/fsverity jmorris-security/next-testing v5.16-rc3 next-20211129]
[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]
url: https://github.com/0day-ci/linux/commits/Mimi-Zohar/ima-support-fs-verity-signatures-stored-as/20211130-010506
base: https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git next-integrity
config: i386-randconfig-m021-20211129 (https://download.01.org/0day-ci/archive/20211130/202111300735.5PPAuPnJ-lkp@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/305ad3bb36a5bd51fe30b33f623eaf165e2efb13
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Mimi-Zohar/ima-support-fs-verity-signatures-stored-as/20211130-010506
git checkout 305ad3bb36a5bd51fe30b33f623eaf165e2efb13
# save the config file to linux build tree
mkdir build_dir
make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
ld: fs/verity/measure.o: in function `fsverity_measure':
fs/verity/measure.c:89: undefined reference to `hash_algo_name'
>> ld: fs/verity/measure.c:104: undefined reference to `hash_digest_size'
ld: fs/verity/measure.c:104: undefined reference to `hash_algo_name'
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Mimi, I love your patch! Yet something to improve: [auto build test ERROR on zohar-integrity/next-integrity] [also build test ERROR on v5.16-rc3 next-20211129] [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] url: https://github.com/0day-ci/linux/commits/Mimi-Zohar/ima-support-fs-verity-signatures-stored-as/20211130-010506 base: https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git next-integrity config: arm64-randconfig-r032-20211129 (https://download.01.org/0day-ci/archive/20211130/202111300736.Vz00z1Kd-lkp@intel.com/config) compiler: aarch64-linux-gcc (GCC) 11.2.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/0day-ci/linux/commit/305ad3bb36a5bd51fe30b33f623eaf165e2efb13 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Mimi-Zohar/ima-support-fs-verity-signatures-stored-as/20211130-010506 git checkout 305ad3bb36a5bd51fe30b33f623eaf165e2efb13 # save the config file to linux build tree mkdir build_dir COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arm64 SHELL=/bin/bash If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): aarch64-linux-ld: fs/verity/measure.o: in function `fsverity_measure': >> (.text+0x968): undefined reference to `hash_algo_name' aarch64-linux-ld: fs/verity/measure.o: relocation R_AARCH64_ADR_PREL_PG_HI21 against symbol `hash_algo_name' which may bind externally can not be used when making a shared object; recompile with -fPIC (.text+0x968): dangerous relocation: unsupported relocation >> aarch64-linux-ld: (.text+0x96c): undefined reference to `hash_algo_name' >> aarch64-linux-ld: (.text+0xa8c): undefined reference to `hash_digest_size' aarch64-linux-ld: fs/verity/measure.o: relocation R_AARCH64_ADR_PREL_PG_HI21 against symbol `hash_digest_size' which may bind externally can not be used when making a shared object; recompile with -fPIC (.text+0xa8c): dangerous relocation: unsupported relocation aarch64-linux-ld: (.text+0xa9c): undefined reference to `hash_digest_size' --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Generally looks fine. A few nits below: On Mon, Nov 29, 2021 at 12:00:54PM -0500, Mimi Zohar wrote: > Define a function named fsverity_measure() to return the verity file digest > and the associated hash algorithm (enum hash_algo). > > Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> > --- > fs/verity/fsverity_private.h | 6 ----- > fs/verity/measure.c | 49 ++++++++++++++++++++++++++++++++++++ > include/linux/fsverity.h | 17 +++++++++++++ > 3 files changed, 66 insertions(+), 6 deletions(-) > > diff --git a/fs/verity/fsverity_private.h b/fs/verity/fsverity_private.h > index a7920434bae5..54c5f0993541 100644 > --- a/fs/verity/fsverity_private.h > +++ b/fs/verity/fsverity_private.h > @@ -26,12 +26,6 @@ struct ahash_request; > */ > #define FS_VERITY_MAX_LEVELS 8 > > -/* > - * Largest digest size among all hash algorithms supported by fs-verity. > - * Currently assumed to be <= size of fsverity_descriptor::root_hash. > - */ > -#define FS_VERITY_MAX_DIGEST_SIZE SHA512_DIGEST_SIZE The include of sha2.h should be removed from this file. > +/** > + * fsverity_measure() - get a verity file's digest > + * @inode: inode to get digest of > + * @digest: pointer to the digest > + * @alg: pointer to the hash algorithm enumeration It should be made clear that @digest and @alg are output, for example: * @digest: (out) pointer to the digest * @alg: (out) pointer to the hash algorithm enumeration > + * Return the file hash algorithm, digest size, and digest of an fsverity > + * protected file. The digest size is implied, not returned. > + > + if (!strcmp(hash_alg->name, hash_algo_name[i])) { As the kernel test robot pointed out, this creates a dependency on CRYPTO_HASH_INFO. So FS_VERITY will need to select CRYPTO_HASH_INFO. - Eric
Hi Mimi, On 11/29/2021 6:19 PM, Eric Biggers wrote: > Generally looks fine. A few nits below: > > On Mon, Nov 29, 2021 at 12:00:54PM -0500, Mimi Zohar wrote: >> Define a function named fsverity_measure() to return the verity file digest >> and the associated hash algorithm (enum hash_algo). >> >> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> >> --- >> fs/verity/fsverity_private.h | 6 ----- >> fs/verity/measure.c | 49 ++++++++++++++++++++++++++++++++++++ >> include/linux/fsverity.h | 17 +++++++++++++ >> 3 files changed, 66 insertions(+), 6 deletions(-) >> >> diff --git a/fs/verity/fsverity_private.h b/fs/verity/fsverity_private.h >> index a7920434bae5..54c5f0993541 100644 >> --- a/fs/verity/fsverity_private.h >> +++ b/fs/verity/fsverity_private.h >> @@ -26,12 +26,6 @@ struct ahash_request; >> */ >> #define FS_VERITY_MAX_LEVELS 8 >> >> -/* >> - * Largest digest size among all hash algorithms supported by fs-verity. >> - * Currently assumed to be <= size of fsverity_descriptor::root_hash. >> - */ >> -#define FS_VERITY_MAX_DIGEST_SIZE SHA512_DIGEST_SIZE > > The include of sha2.h should be removed from this file. > >> +/** >> + * fsverity_measure() - get a verity file's digest nit: The function name seems to suggest it is measuring the fs-verity file's digest. Since it is reading the file's digest: fsverity_read_digest() or fsverity_read_measure()? -lakshmi >> + * @inode: inode to get digest of >> + * @digest: pointer to the digest >> + * @alg: pointer to the hash algorithm enumeration > > It should be made clear that @digest and @alg are output, for example: > > * @digest: (out) pointer to the digest > * @alg: (out) pointer to the hash algorithm enumeration > >> + * Return the file hash algorithm, digest size, and digest of an fsverity >> + * protected file. > > The digest size is implied, not returned. > >> + >> + if (!strcmp(hash_alg->name, hash_algo_name[i])) { > > As the kernel test robot pointed out, this creates a dependency on > CRYPTO_HASH_INFO. So FS_VERITY will need to select CRYPTO_HASH_INFO. > > - Eric >
On Mon, Nov 29, 2021 at 09:33:29PM -0800, Lakshmi Ramasubramanian wrote: > > > +/** > > > + * fsverity_measure() - get a verity file's digest > nit: The function name seems to suggest it is measuring the fs-verity file's > digest. Since it is reading the file's digest: fsverity_read_digest() or > fsverity_read_measure()? I suggest fsverity_get_digest(). "read" is misleading because it's not being read from disk. - Eric
diff --git a/fs/verity/fsverity_private.h b/fs/verity/fsverity_private.h index a7920434bae5..54c5f0993541 100644 --- a/fs/verity/fsverity_private.h +++ b/fs/verity/fsverity_private.h @@ -26,12 +26,6 @@ struct ahash_request; */ #define FS_VERITY_MAX_LEVELS 8 -/* - * Largest digest size among all hash algorithms supported by fs-verity. - * Currently assumed to be <= size of fsverity_descriptor::root_hash. - */ -#define FS_VERITY_MAX_DIGEST_SIZE SHA512_DIGEST_SIZE - /* A hash algorithm supported by fs-verity */ struct fsverity_hash_alg { struct crypto_ahash *tfm; /* hash tfm, allocated on demand */ diff --git a/fs/verity/measure.c b/fs/verity/measure.c index f0d7b30c62db..98d8f6f2a2be 100644 --- a/fs/verity/measure.c +++ b/fs/verity/measure.c @@ -57,3 +57,52 @@ int fsverity_ioctl_measure(struct file *filp, void __user *_uarg) return 0; } EXPORT_SYMBOL_GPL(fsverity_ioctl_measure); + +/** + * fsverity_measure() - get a verity file's digest + * @inode: inode to get digest of + * @digest: pointer to the digest + * @alg: pointer to the hash algorithm enumeration + * + * Return the file hash algorithm, digest size, and digest of an fsverity + * protected file. + * + * Return: 0 on success, -errno on failure + */ +int fsverity_measure(struct inode *inode, u8 digest[FS_VERITY_MAX_DIGEST_SIZE], + enum hash_algo *alg) +{ + const struct fsverity_info *vi; + const struct fsverity_hash_alg *hash_alg; + int i; + + vi = fsverity_get_info(inode); + if (!vi) + return -ENODATA; /* not a verity file */ + + hash_alg = vi->tree_params.hash_alg; + memset(digest, 0, FS_VERITY_MAX_DIGEST_SIZE); + *alg = HASH_ALGO__LAST; + + /* convert hash algorithm to hash_algo_name */ + for (i = 0; i < HASH_ALGO__LAST; i++) { + pr_debug("name %s hash_algo_name[%d] %s\n", + hash_alg->name, i, hash_algo_name[i]); + + if (!strcmp(hash_alg->name, hash_algo_name[i])) { + *alg = i; + break; + } + } + + /* Shouldn't happen */ + if (*alg == HASH_ALGO__LAST) + return -EINVAL; + + memcpy(digest, vi->file_digest, hash_alg->digest_size); + + pr_debug("file digest:%s %*phN\n", hash_algo_name[*alg], + hash_digest_size[*alg], digest); + + return 0; +} diff --git a/include/linux/fsverity.h b/include/linux/fsverity.h index b568b3c7d095..11006b60713b 100644 --- a/include/linux/fsverity.h +++ b/include/linux/fsverity.h @@ -12,8 +12,16 @@ #define _LINUX_FSVERITY_H #include <linux/fs.h> +#include <crypto/hash_info.h> +#include <crypto/sha2.h> #include <uapi/linux/fsverity.h> +/* + * Largest digest size among all hash algorithms supported by fs-verity. + * Currently assumed to be <= size of fsverity_descriptor::root_hash. + */ +#define FS_VERITY_MAX_DIGEST_SIZE SHA512_DIGEST_SIZE + /* Verity operations for filesystems */ struct fsverity_operations { @@ -131,6 +139,8 @@ int fsverity_ioctl_enable(struct file *filp, const void __user *arg); /* measure.c */ int fsverity_ioctl_measure(struct file *filp, void __user *arg); +int fsverity_measure(struct inode *inode, u8 digest[FS_VERITY_MAX_DIGEST_SIZE], + enum hash_algo *alg); /* open.c */ @@ -170,6 +180,13 @@ static inline int fsverity_ioctl_measure(struct file *filp, void __user *arg) return -EOPNOTSUPP; } +static inline int fsverity_measure(struct inode *inode, + u8 digest[FS_VERITY_MAX_DIGEST_SIZE], + enum hash_algo *alg) +{ + return -EOPNOTSUPP; +} + /* open.c */ static inline int fsverity_file_open(struct inode *inode, struct file *filp)
Define a function named fsverity_measure() to return the verity file digest and the associated hash algorithm (enum hash_algo). Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> --- fs/verity/fsverity_private.h | 6 ----- fs/verity/measure.c | 49 ++++++++++++++++++++++++++++++++++++ include/linux/fsverity.h | 17 +++++++++++++ 3 files changed, 66 insertions(+), 6 deletions(-)