diff mbox series

[v2,5/9] evm: Use the inode holding the metadata to calculate metadata hash

Message ID 20240205182506.3569743-6-stefanb@linux.ibm.com (mailing list archive)
State Handled Elsewhere
Headers show
Series evm: Support signatures on stacked filesystem | expand

Commit Message

Stefan Berger Feb. 5, 2024, 6:25 p.m. UTC
Changes to file attributes (mode bits, uid, gid) on the lower layer are
not taken into account when d_backing_inode() is used when a file is
accessed on the overlay layer and this file has not yet been copied up.
This is because d_backing_inode() does not return the real inode of the
lower layer but instead returns the backing inode which in this case
holds wrong file attributes. Further, when CONFIG_OVERLAY_FS_METACOPY is
enabled and a copy-up is triggered due to file metadata changes, then
the metadata are held by the backing inode while the data are still held
by the real inode. Therefore, use d_inode(d_real(dentry, D_REAL_METADATA))
to get to the inode holding the file's metadata and use it to calculate
the metadata hash with.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 security/integrity/evm/evm_crypto.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Amir Goldstein Feb. 6, 2024, 3:33 p.m. UTC | #1
On Mon, Feb 5, 2024 at 8:25 PM Stefan Berger <stefanb@linux.ibm.com> wrote:
>
> Changes to file attributes (mode bits, uid, gid) on the lower layer are
> not taken into account when d_backing_inode() is used when a file is
> accessed on the overlay layer and this file has not yet been copied up.
> This is because d_backing_inode() does not return the real inode of the
> lower layer but instead returns the backing inode which in this case
> holds wrong file attributes. Further, when CONFIG_OVERLAY_FS_METACOPY is
> enabled and a copy-up is triggered due to file metadata changes, then
> the metadata are held by the backing inode while the data are still held
> by the real inode. Therefore, use d_inode(d_real(dentry, D_REAL_METADATA))
> to get to the inode holding the file's metadata and use it to calculate
> the metadata hash with.
>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>

Acked-by: Amir Goldstein <amir73il@gmail.com>

> ---
>  security/integrity/evm/evm_crypto.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
> index b1ffd4cc0b44..51e24a75742c 100644
> --- a/security/integrity/evm/evm_crypto.c
> +++ b/security/integrity/evm/evm_crypto.c
> @@ -223,7 +223,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
>                                  size_t req_xattr_value_len,
>                                  uint8_t type, struct evm_digest *data)
>  {
> -       struct inode *inode = d_backing_inode(dentry);
> +       struct inode *inode = d_inode(d_real(dentry, D_REAL_METADATA));
>         struct xattr_list *xattr;
>         struct shash_desc *desc;
>         size_t xattr_size = 0;
> --
> 2.43.0
>
kernel test robot Feb. 6, 2024, 6:22 p.m. UTC | #2
Hi Stefan,

kernel test robot noticed the following build errors:

[auto build test ERROR on zohar-integrity/next-integrity]
[also build test ERROR on pcmoore-selinux/next linus/master v6.8-rc3 next-20240206]
[cannot apply to mszeredi-vfs/overlayfs-next mszeredi-vfs/next]
[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/Stefan-Berger/ima-Rename-backing_inode-to-real_inode/20240206-022848
base:   https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git next-integrity
patch link:    https://lore.kernel.org/r/20240205182506.3569743-6-stefanb%40linux.ibm.com
patch subject: [PATCH v2 5/9] evm: Use the inode holding the metadata to calculate metadata hash
config: x86_64-rhel-8.3 (https://download.01.org/0day-ci/archive/20240207/202402070220.eYpQ6zcm-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240207/202402070220.eYpQ6zcm-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202402070220.eYpQ6zcm-lkp@intel.com/

All errors (new ones prefixed by >>):

   security/integrity/evm/evm_crypto.c: In function 'evm_calc_hmac_or_hash':
>> security/integrity/evm/evm_crypto.c:226:54: error: 'D_REAL_METADATA' undeclared (first use in this function)
     226 |         struct inode *inode = d_inode(d_real(dentry, D_REAL_METADATA));
         |                                                      ^~~~~~~~~~~~~~~
   security/integrity/evm/evm_crypto.c:226:54: note: each undeclared identifier is reported only once for each function it appears in


vim +/D_REAL_METADATA +226 security/integrity/evm/evm_crypto.c

   212	
   213	/*
   214	 * Calculate the HMAC value across the set of protected security xattrs.
   215	 *
   216	 * Instead of retrieving the requested xattr, for performance, calculate
   217	 * the hmac using the requested xattr value. Don't alloc/free memory for
   218	 * each xattr, but attempt to re-use the previously allocated memory.
   219	 */
   220	static int evm_calc_hmac_or_hash(struct dentry *dentry,
   221					 const char *req_xattr_name,
   222					 const char *req_xattr_value,
   223					 size_t req_xattr_value_len,
   224					 uint8_t type, struct evm_digest *data)
   225	{
 > 226		struct inode *inode = d_inode(d_real(dentry, D_REAL_METADATA));
   227		struct xattr_list *xattr;
   228		struct shash_desc *desc;
   229		size_t xattr_size = 0;
   230		char *xattr_value = NULL;
   231		int error;
   232		int size, user_space_size;
   233		bool ima_present = false;
   234	
   235		if (!(inode->i_opflags & IOP_XATTR) ||
   236		    inode->i_sb->s_user_ns != &init_user_ns)
   237			return -EOPNOTSUPP;
   238	
   239		desc = init_desc(type, data->hdr.algo);
   240		if (IS_ERR(desc))
   241			return PTR_ERR(desc);
   242	
   243		data->hdr.length = crypto_shash_digestsize(desc->tfm);
   244	
   245		error = -ENODATA;
   246		list_for_each_entry_lockless(xattr, &evm_config_xattrnames, list) {
   247			bool is_ima = false;
   248	
   249			if (strcmp(xattr->name, XATTR_NAME_IMA) == 0)
   250				is_ima = true;
   251	
   252			/*
   253			 * Skip non-enabled xattrs for locally calculated
   254			 * signatures/HMACs.
   255			 */
   256			if (type != EVM_XATTR_PORTABLE_DIGSIG && !xattr->enabled)
   257				continue;
   258	
   259			if ((req_xattr_name && req_xattr_value)
   260			    && !strcmp(xattr->name, req_xattr_name)) {
   261				error = 0;
   262				crypto_shash_update(desc, (const u8 *)req_xattr_value,
   263						     req_xattr_value_len);
   264				if (is_ima)
   265					ima_present = true;
   266	
   267				dump_security_xattr(req_xattr_name,
   268						    req_xattr_value,
   269						    req_xattr_value_len);
   270				continue;
   271			}
   272			size = vfs_getxattr_alloc(&nop_mnt_idmap, dentry, xattr->name,
   273						  &xattr_value, xattr_size, GFP_NOFS);
   274			if (size == -ENOMEM) {
   275				error = -ENOMEM;
   276				goto out;
   277			}
   278			if (size < 0)
   279				continue;
   280	
   281			user_space_size = vfs_getxattr(&nop_mnt_idmap, dentry,
   282						       xattr->name, NULL, 0);
   283			if (user_space_size != size)
   284				pr_debug("file %s: xattr %s size mismatch (kernel: %d, user: %d)\n",
   285					 dentry->d_name.name, xattr->name, size,
   286					 user_space_size);
   287			error = 0;
   288			xattr_size = size;
   289			crypto_shash_update(desc, (const u8 *)xattr_value, xattr_size);
   290			if (is_ima)
   291				ima_present = true;
   292	
   293			dump_security_xattr(xattr->name, xattr_value, xattr_size);
   294		}
   295		hmac_add_misc(desc, inode, type, data->digest);
   296	
   297		/* Portable EVM signatures must include an IMA hash */
   298		if (type == EVM_XATTR_PORTABLE_DIGSIG && !ima_present)
   299			error = -EPERM;
   300	out:
   301		kfree(xattr_value);
   302		kfree(desc);
   303		return error;
   304	}
   305
diff mbox series

Patch

diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index b1ffd4cc0b44..51e24a75742c 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -223,7 +223,7 @@  static int evm_calc_hmac_or_hash(struct dentry *dentry,
 				 size_t req_xattr_value_len,
 				 uint8_t type, struct evm_digest *data)
 {
-	struct inode *inode = d_backing_inode(dentry);
+	struct inode *inode = d_inode(d_real(dentry, D_REAL_METADATA));
 	struct xattr_list *xattr;
 	struct shash_desc *desc;
 	size_t xattr_size = 0;