LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Richard Guy Briggs <rgb@redhat.com>
To: Kees Cook <keescook@chromium.org>
Cc: Linux-Audit Mailing List <linux-audit@redhat.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Eric Paris <eparis@redhat.com>, Paul Moore <paul@paul-moore.com>,
	Steve Grubb <sgrubb@redhat.com>
Subject: Re: [PATCH ghak21 V4 1/2] audit: remove path param from link denied function
Date: Mon, 16 Apr 2018 13:55:04 -0400	[thread overview]
Message-ID: <20180416175504.ldn62amrk7oxogia@madcap2.tricolour.ca> (raw)
In-Reply-To: <CAGXu5jJrCntv1fGLtVALyh5+UGdn5DvCkM230Vn7QGutY8k1og@mail.gmail.com>

On 2018-04-16 10:55, Kees Cook wrote:
> On Wed, Mar 21, 2018 at 1:42 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
> > In commit 45b578fe4c3cade6f4ca1fc934ce199afd857edc
> > ("audit: link denied should not directly generate PATH record")
> > the need for the struct path *link parameter was removed.
> > Remove the now useless struct path argument.
> >
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > ---
> >  fs/namei.c            | 4 ++--
> >  include/linux/audit.h | 6 ++----
> >  kernel/audit.c        | 3 +--
> >  3 files changed, 5 insertions(+), 8 deletions(-)
> >
> > diff --git a/fs/namei.c b/fs/namei.c
> > index 9cc91fb..e3682bb 100644
> > --- a/fs/namei.c
> > +++ b/fs/namei.c
> > @@ -945,7 +945,7 @@ static inline int may_follow_link(struct nameidata *nd)
> >         if (nd->flags & LOOKUP_RCU)
> >                 return -ECHILD;
> >
> > -       audit_log_link_denied("follow_link", &nd->stack[0].link);
> > +       audit_log_link_denied("follow_link");
> >         return -EACCES;
> >  }
> >
> > @@ -1011,7 +1011,7 @@ static int may_linkat(struct path *link)
> >         if (safe_hardlink_source(inode) || inode_owner_or_capable(inode))
> >                 return 0;
> >
> > -       audit_log_link_denied("linkat", link);
> > +       audit_log_link_denied("linkat");
> >         return -EPERM;
> >  }
> 
> This removed the "link" details in both cases, and then commit
> ea841bafda3f ("audit: add refused symlink to audit_names") added back
> one of them:
> 
> > diff --git a/fs/namei.c b/fs/namei.c
> > index e3682bb72cb5..5f8e8e2732e1 100644
> > --- a/fs/namei.c
> > +++ b/fs/namei.c
> > @@ -945,6 +945,7 @@ static inline int may_follow_link(struct nameidata *nd)
> >         if (nd->flags & LOOKUP_RCU)
> >                 return -ECHILD;
> >
> > +       audit_inode(nd->name, nd->stack[0].link.dentry, 0);
> >         audit_log_link_denied("follow_link");
> >         return -EACCES;
> >  }
> 
> Why remove it in the first place, and why add it back open-coded in
> only one place?

In both cases, the extra PATH record didn't make sense with the SYSCALL
record "items=" field and conflicted with another PATH record's "item="
field.

All the necessary details were already available in the hardlink case
via two PATH records associated with the SYSCALL record.

There were two conflicting PATH records in the symlink case, one of them
incomplete, so it made more sense to make the existing one complete.

> -Kees

- 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

  reply	other threads:[~2018-04-16 18:01 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-21  8:42 [PATCH ghak21 V4 0/2] audit: address ANOM_LINK excess records Richard Guy Briggs
2018-03-21  8:42 ` [PATCH ghak21 V4 1/2] audit: remove path param from link denied function Richard Guy Briggs
2018-03-21 15:21   ` Paul Moore
2018-04-16 17:55   ` Kees Cook
2018-04-16 17:55     ` Richard Guy Briggs [this message]
2018-03-21  8:42 ` [PATCH ghak21 V4 2/2] audit: add refused symlink to audit_names Richard Guy Briggs
2018-03-21 15:49   ` Paul Moore

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180416175504.ldn62amrk7oxogia@madcap2.tricolour.ca \
    --to=rgb@redhat.com \
    --cc=eparis@redhat.com \
    --cc=keescook@chromium.org \
    --cc=linux-audit@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=paul@paul-moore.com \
    --cc=sgrubb@redhat.com \
    --subject='Re: [PATCH ghak21 V4 1/2] audit: remove path param from link denied function' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).