Date:   Thu, 9 Feb 2023 16:53:12 -0500
From:   Richard Guy Briggs <rgb@redhat.com>
To:     Paul Moore <paul@paul-moore.com>
Cc:     Linux-Audit Mailing List <linux-audit@redhat.com>,
        LKML <linux-kernel@vger.kernel.org>, io-uring@vger.kernel.org,
        Eric Paris <eparis@parisplace.org>,
        Steve Grubb <sgrubb@redhat.com>,
        Christian Brauner <brauner@kernel.org>,
        Stefan Roesch <shr@fb.com>
Subject: Re: [PATCH v2] io_uring,audit: don't log IORING_OP_MADVISE
Message-ID: <Y+VrSLZKZoAGikUS@madcap2.tricolour.ca>
References: <b5dfdcd541115c86dbc774aa9dd502c964849c5f.1675282642.git.rgb@redhat.com>
 <CAHC9VhS0rPfkwUT1WMfqsTF-qYXdbbhHAfVPs=d3ZQVgbXBHnw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAHC9VhS0rPfkwUT1WMfqsTF-qYXdbbhHAfVPs=d3ZQVgbXBHnw@mail.gmail.com>
Precedence: bulk

On 2023-02-01 16:18, Paul Moore wrote:
> On Wed, Feb 1, 2023 at 3:34 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> > fadvise and madvise both provide hints for caching or access pattern for
> > file and memory respectively.  Skip them.
> 
> You forgot to update the first sentence in the commit description :/

I didn't forget.  I updated that sentence to reflect the fact that the
two should be treated similarly rather than differently.

> I'm still looking for some type of statement that you've done some
> homework on the IORING_OP_MADVISE case to ensure that it doesn't end
> up calling into the LSM, see my previous emails on this.  I need more
> than "Steve told me to do this".
> 
> I basically just want to see that some care and thought has gone into
> this patch to verify it is correct and good.

Steve suggested I look into a number of iouring ops.  I looked at the
description code and agreed that it wasn't necessary to audit madvise.
The rationale for fadvise was detemined to have been conflated with
fallocate and subsequently dropped.  Steve also suggested a number of
others and after investigation I decided that their current state was
correct.  *getxattr you've advised against, so it was dropped.  It
appears fewer modifications were necessary than originally suspected.

> > Fixes: 5bd2182d58e9 ("audit,io_uring,io-wq: add some basic audit support to io_uring")
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > ---
> > changelog
> > v2:
> > - drop *GETXATTR patch
> > - drop FADVISE hunk
> >
> >  io_uring/opdef.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/io_uring/opdef.c b/io_uring/opdef.c
> > index 3aa0d65c50e3..d3f36c633ceb 100644
> > --- a/io_uring/opdef.c
> > +++ b/io_uring/opdef.c
> > @@ -312,6 +312,7 @@ const struct io_op_def io_op_defs[] = {
> >                 .issue                  = io_fadvise,
> >         },
> >         [IORING_OP_MADVISE] = {
> > +               .audit_skip             = 1,
> >                 .name                   = "MADVISE",
> >                 .prep                   = io_madvise_prep,
> >                 .issue                  = io_madvise,
> > --
> > 2.27.0
> 
> -- 
> paul-moore.com
> 

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635