lkml.org 
[lkml]   [2024]   [Apr]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    SubjectRe: [RFC PATCH v2 2/2] ima: Fix detection of read/write violations on stacked filesystems
    From
    Date
    On Mon, 2024-04-22 at 11:06 -0400, Stefan Berger 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.
    >
    > Convert the current code so that it can handle the normal case as well
    > as the stacked filesystem case. Therefore, use d_real with parameter
    > D_REAL_FILEDATA to get the next dentry holding the file data. On a normal
    > filesystem this would be the dentry of the file and on a stacked filesystem
    > this could be an upper or lower dentry. Check the dentry's inode for
    > writes and if it has any issue the violation. Otherwise continue onto the
    > next dentry given the current dentry by again calling d_real. On a normal
    > filesystem this would return the same dentry as before and on a stacked
    > filesystem it would return the next-level dentry, so either the upper
    > or lower dentry of the next lower layer.

    I have a question. What happens for the opposite, when you open an
    overlayfs file for write, and it was already opened and measured in the
    lower layers?

    If IMA was invoked for the lower layers, the ToMToU detection would be
    detected. But it does not seem the case:

    struct file *backing_file_open(const struct path *user_path, int flags,
    const struct path *real_path,
    const struct cred *cred)
    {

    [...]

    error = vfs_open(real_path, f);
    if (error) {
    fput(f);
    f = ERR_PTR(error);
    }

    return f;

    security_file_post_open(), which invokes IMA, is called after
    vfs_open() and only in do_open().

    Also, consider that ima_iint_get() and ima_rdwr_violation_check() are
    called with the inode lock held. If you access different inodes, the
    locking scheme changes.

    Thanks

    Roberto

    > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
    > ---
    > security/integrity/ima/ima_main.c | 21 ++++++++++++++++-----
    > 1 file changed, 16 insertions(+), 5 deletions(-)
    >
    > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
    > index f04f43af651c..7d727c448dc7 100644
    > --- a/security/integrity/ima/ima_main.c
    > +++ b/security/integrity/ima/ima_main.c
    > @@ -122,7 +122,9 @@ static void ima_rdwr_violation_check(struct file *file,
    > char *filename)
    > {
    > 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 +136,20 @@ 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);
    > +
    > + 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);
    > }
    >
    > if (!send_tomtou && !send_writers)


    \
     
     \ /
      Last update: 2024-04-22 18:16    [W:3.534 / U:0.244 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site