| Message ID | 20240205182506.3569743-6-stefanb@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | evm: Support signatures on stacked filesystem | expand |
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 >
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 --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;
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(-)