On 4/12/24 14:08, Amir Goldstein wrote:
On Fri, Apr 12, 2024 at 5:01 PM Stefan Berger <stefanb@xxxxxxxxxxxxx> wrote:
On a stacked filesystem, when one process opens the file holding a file's
data (e.g., on upper or lower layer on overlayfs) then issue a violation
when another process opens the file for reading on the top layer (overlay
layer on overlayfs). This then provides similar behavior to the existing
case where a violation is generated when one process opens a file for
writing and another one opens the same file for reading. On stacked
filesystem also search all the lower layers for relevant files opened for
writing and issue the violation if one is found.
Signed-off-by: Stefan Berger <stefanb@xxxxxxxxxxxxx>
---
security/integrity/ima/ima_main.c | 27 ++++++++++++++++++++++-----
1 file changed, 22 insertions(+), 5 deletions(-)
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index f04f43af651c..590dd9d5d99a 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -121,8 +121,11 @@ static void ima_rdwr_violation_check(struct file *file,
const char **pathname,
char *filename)
{
+ struct inode *real_inode = d_real_inode(file_dentry(file));
struct inode *inode = file_inode(file);
+ struct dentry *fd_dentry, *d;
fmode_t mode = file->f_mode;
+ struct inode *fd_inode;
bool send_tomtou = false, send_writers = false;
if (mode & FMODE_WRITE) {
@@ -134,11 +137,25 @@ static void ima_rdwr_violation_check(struct file *file,
&iint->atomic_flags))
send_tomtou = true;
}
- } else {
- if (must_measure)
- set_bit(IMA_MUST_MEASURE, &iint->atomic_flags);
- if (inode_is_open_for_write(inode) && must_measure)
- send_writers = true;
+ } else if (must_measure) {
+ set_bit(IMA_MUST_MEASURE, &iint->atomic_flags);
+
+ if (inode == real_inode) {
+ if (inode_is_open_for_write(inode))
+ send_writers = true;
+ } else {
+ d = d_real(file_dentry(file), D_REAL_FILEDATA);
+ do {
+ fd_dentry = d;
+ fd_inode = d_inode(fd_dentry);
+ if (inode_is_open_for_write(fd_inode)) {
+ send_writers = true;
+ break;
+ }
+ /* next layer of stacked fs */
+ d = d_real(fd_dentry, D_REAL_FILEDATA);
+ } while (d != fd_dentry);
+ }
The idea of digging though ovl layers feels wrong to me.
I have a couple of test cases that expect violations to be logged. One
test case has 2 overlay filesystems stacked on top of each other (lower
= A, upper = B) and it passes those test cases when for example
- opening the file on lower on 'A' for writing
- opening the file on overlay layer on 'B' for reading
OR
- opening the file on overlay layer on 'A' (= lower layer of 'B') for
writing
- opening the file on overlay layer on 'B' for reading
After causing a copy-up only the following test case causes a violation
to be logged:
- opening the file on upper on 'B' for writing
- opening the file on overlay layer on 'B' for reading
No violation will the be logged for example for:
- opening the file on overlay layer on 'A' (= lower of 'B') for writing
- opening the file on overlay layer on 'B' for reading
As Miklos is the designer of overlayfs and its vfs architecture,
I was hoping that this would be sufficiently generic to work with
potential future stacked filesystems as well that would need to also
provide support for D_REAL_FILEDATA.
I am deferring the call about adding this interface to Miklos.
Thanks,
Amir.
