On 1/31/24 10:54, Amir Goldstein wrote:
On Wed, Jan 31, 2024 at 4:40 PM Stefan Berger <stefanb@xxxxxxxxxxxxx> wrote:
On 1/31/24 08:16, Amir Goldstein wrote:
On Wed, Jan 31, 2024 at 4:11 AM Stefan Berger <stefanb@xxxxxxxxxxxxx> wrote:
On 1/30/24 16:46, Stefan Berger wrote:
Changes to the file attribute (mode bits, uid, gid) on the lower layer
are not take 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 holds old file
attributes. When the old file attributes are used for calculating the
metadata hash then the expected hash is calculated and the file then
mistakenly passes signature verification. Therefore, use d_real_inode()
which returns the inode of the lower layer for as long as the file has
not been copied up and returns the upper layer's inode otherwise.
Signed-off-by: Stefan Berger <stefanb@xxxxxxxxxxxxx>
---
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..2e48fe54e899 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_real_inode(dentry);
struct xattr_list *xattr;
struct shash_desc *desc;
size_t xattr_size = 0;
We need this patch when NOT activating CONFIG_OVERLAY_FS_METACOPY but
when setting CONFIG_OVERLAY_FS_METACOPY=y it has to be reverted... I am
not sure what the solution is.
I think d_real_inode() does not work correctly for all its current users for
a metacopy file.
I think the solution is to change d_real_inode() to return the data inode
and add another helper to get the metadata inode if needed.
I will post some patches for it.
I thought that we may have to go through vfs_getattr() but even better
if we don't because we don't have the file *file anywhere 'near'.
However, I must say that I do not know if evm_calc_hmac_or_hash()
needs the lower data inode, the upper metadata inode or both.
What it needs are data structures with mode bits, uid, and gid that stat
in userspace would show.
With or without metacopy enabled, an overlay inode st_uid st_gid st_mode
are always taken from the upper most inode which is what d_real_inode()
currently returns, so I do not understand what the problem is.
I have testcases that work fine with this series when
CONFIG_OVERLAY_FS_METACOPY is not active. Once I activate this then a
test case that changes a file's gid on the overlay layer from 0 to '12'
while causing a copy-up allows a file to execute even thugh it should
not execute. The reason is because d_real_inode(dentry)->i_guid shows
the '0' while d_backing_dentry(dentry)->i_guid shows '12'. User space
stat also shows '12' as expected.
Just saw your other email, will try that now ...
