On Thursday, February 9, 2023 5:37:22 PM EST Paul Moore wrote:
 On Thu, Feb 9, 2023 at 4:53 PM Richard Guy Briggs
<rgb(a)redhat.com> wrote:
 > On 2023-02-01 16:18, Paul Moore wrote:
 > > On Wed, Feb 1, 2023 at 3:34 PM Richard Guy Briggs <rgb(a)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.
 
 Ooookay.  Can we at least agree that the commit description should be
 rephrased to make it clear that the patch only adjusts madvise?  Right
 now I read the commit description and it sounds like you are adjusting
 the behavior for both fadvise and madvise in this patch, which is not
 true.
 
 > > 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.
 
 My concern is that three of the four changes you initially proposed
 were rejected, which gives me pause about the fourth.  You mention
 that based on your reading of madvise's description you feel auditing
 isn't necessary - and you may be right - but based on our experience
 so far with this patchset I would like to hear that you have properly
 investigated all of the madvise code paths, and I would like that in
 the commit description. 
I think you're being unnecessarily hard on this. Yes, the commit message 
might be touched up. But madvise is advisory in nature. It is not security 
relevant. And a grep through the security directory doesn't turn up any 
hooks.
-Steve