Received-SPF: pass (google.com: domain of linux-kernel+bounces-143174-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240412140122.2607743-1-stefanb@linux.ibm.com> <20240412140122.2607743-3-stefanb@linux.ibm.com>
In-Reply-To: <20240412140122.2607743-3-stefanb@linux.ibm.com>
From: Amir Goldstein <amir73il@gmail.com>
Date: Fri, 12 Apr 2024 21:08:17 +0300
Message-ID: <CAOQ4uxjDQO91cjA0sgyPStkwc_7+NxAOhyve94qUvXSM3ytk1g@mail.gmail.com>
Subject: Re: [RFC 2/2] ima: Fix detection of read/write violations on stacked filesystems
To: Stefan Berger <stefanb@linux.ibm.com>
Cc: linux-integrity@vger.kernel.org, linux-unionfs@vger.kernel.org, 
	linux-kernel@vger.kernel.org, zohar@linux.ibm.com, roberto.sassu@huawei.com, 
	miklos@szeredi.hu, Christian Brauner <brauner@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Fri, Apr 12, 2024 at 5:01=E2=80=AFPM Stefan Berger <stefanb@linux.ibm.co=
m> 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@linux.ibm.com>
> ---
>  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/i=
ma_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 *fi=
le,
>                                      const char **pathname,
>                                      char *filename)
>  {
> +       struct inode *real_inode =3D d_real_inode(file_dentry(file));
>         struct inode *inode =3D file_inode(file);
> +       struct dentry *fd_dentry, *d;
>         fmode_t mode =3D file->f_mode;
> +       struct inode *fd_inode;
>         bool send_tomtou =3D false, send_writers =3D false;
>
>         if (mode & FMODE_WRITE) {
> @@ -134,11 +137,25 @@ static void ima_rdwr_violation_check(struct file *f=
ile,
>                                                 &iint->atomic_flags))
>                                 send_tomtou =3D true;
>                 }
> -       } else {
> -               if (must_measure)
> -                       set_bit(IMA_MUST_MEASURE, &iint->atomic_flags);
> -               if (inode_is_open_for_write(inode) && must_measure)
> -                       send_writers =3D true;
> +       } else if (must_measure) {
> +               set_bit(IMA_MUST_MEASURE, &iint->atomic_flags);
> +
> +               if (inode =3D=3D real_inode) {
> +                       if (inode_is_open_for_write(inode))
> +                               send_writers =3D true;
> +               } else {
> +                       d =3D d_real(file_dentry(file), D_REAL_FILEDATA);
> +                       do {
> +                               fd_dentry =3D d;
> +                               fd_inode =3D d_inode(fd_dentry);
> +                               if (inode_is_open_for_write(fd_inode)) {
> +                                       send_writers =3D true;
> +                                       break;
> +                               }
> +                               /* next layer of stacked fs */
> +                               d =3D d_real(fd_dentry, D_REAL_FILEDATA);
> +                       } while (d !=3D fd_dentry);
> +               }

The idea of digging though ovl layers feels wrong to me.
As Miklos is the designer of overlayfs and its vfs architecture,
I am deferring the call about adding this interface to Miklos.

Thanks,
Amir.