linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Paul Moore <paul@paul-moore.com>
To: Nathan Chancellor <natechancellor@gmail.com>
Cc: Richard Guy Briggs <rgb@redhat.com>,
	linux-fsdevel@vger.kernel.org, viro@zeniv.linux.org.uk,
	LKML <linux-kernel@vger.kernel.org>,
	Linux-Audit Mailing List <linux-audit@redhat.com>,
	Steve Grubb <sgrubb@redhat.com>, Eric Paris <eparis@redhat.com>
Subject: Re: [PATCH ghak100 V2 2/2] audit: ignore fcaps on umount
Date: Fri, 1 Feb 2019 16:05:22 -0500	[thread overview]
Message-ID: <CAHC9VhTzy-0cPgD01XyRRPcsgtoV2NZ1NEzun388jPmXmEB-7Q@mail.gmail.com> (raw)
In-Reply-To: <20190201204205.GA26179@archlinux-ryzen>

On Fri, Feb 1, 2019 at 3:42 PM Nathan Chancellor
<natechancellor@gmail.com> wrote:
> On Wed, Jan 23, 2019 at 01:35:00PM -0500, Richard Guy Briggs wrote:
> > Don't fetch fcaps when umount2 is called to avoid a process hang while
> > it waits for the missing resource to (possibly never) re-appear.
> >
> > Note the comment above user_path_mountpoint_at():
> >  * A umount is a special case for path walking. We're not actually interested
> >  * in the inode in this situation, and ESTALE errors can be a problem.  We
> >  * simply want track down the dentry and vfsmount attached at the mountpoint
> >  * and avoid revalidating the last component.
> >
> > This can happen on ceph, cifs, 9p, lustre, fuse (gluster) or NFS.
> >
> > Please see the github issue tracker
> > https://github.com/linux-audit/audit-kernel/issues/100
> >
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > ---
> >  fs/namei.c            |  2 +-
> >  fs/namespace.c        |  2 ++
> >  include/linux/audit.h | 15 ++++++++++-----
> >  include/linux/namei.h |  3 +++
> >  kernel/audit.c        | 10 +++++++++-
> >  kernel/audit.h        |  2 +-
> >  kernel/auditsc.c      |  6 +++---
> >  7 files changed, 29 insertions(+), 11 deletions(-)

...

> >  /* Copy inode data into an audit_names. */
> >  void audit_copy_inode(struct audit_names *name, const struct dentry *dentry,
> > -                   struct inode *inode)
> > +                   struct inode *inode, unsigned int flags)
> >  {
> >       name->ino   = inode->i_ino;
> >       name->dev   = inode->i_sb->s_dev;
> > @@ -2120,6 +2124,10 @@ void audit_copy_inode(struct audit_names *name, const struct dentry *dentry,
> >       name->gid   = inode->i_gid;
> >       name->rdev  = inode->i_rdev;
> >       security_inode_getsecid(inode, &name->osid);
> > +     if (flags & AUDIT_INODE_NOEVAL) {
>
> I don't know if this has been reported or if I am missing something but
> on next-20190201, this line causes an error with arm allyesconfig (as
> CONFIG_AUDITSYCALL doesn't get selected):

...

>   CC      kernel/audit.o
> kernel/audit.c: In function 'audit_copy_inode':
> kernel/audit.c:2130:14: error: 'AUDIT_INODE_NOEVAL' undeclared (first use in this function); did you mean 'AUDIT_TYPE_NORMAL'?
>   if (flags & AUDIT_INODE_NOEVAL) {
>               ^~~~~~~~~~~~~~~~~~
>               AUDIT_TYPE_NORMAL
> kernel/audit.c:2130:14: note: each undeclared identifier is reported only once for each function it appears in
> make[2]: *** [scripts/Makefile.build:277: kernel/audit.o] Error 1
> make[1]: *** [Makefile:1699: kernel/audit.o] Error 2
> make: *** [Makefile:296: __build_one_by_one] Error 2

I hadn't seen this reported to the audit list yet, thanks for letting us now.

Richard, please submit a patch to fix this ASAP.  Looking at this, the
obvious fix is to move audit_copy_inode() to auditsc.c, but I'm not
sure if that itself is going to cause problems (it doesn't look like
it).  Actually, thinking out loud, I wonder if we shouldn't move
audit_log_cap(), audit_log_fcaps(), audit_copy_fcaps(), and
audit_log_name() too?

-- 
paul moore
www.paul-moore.com

  reply	other threads:[~2019-02-01 21:05 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-23 18:34 [PATCH ghak100 V2 0/2] audit: avoid umount hangs on missing mount Richard Guy Briggs
2019-01-23 18:34 ` [PATCH ghak100 V2 1/2] audit: more filter PATH records keyed on filesystem magic Richard Guy Briggs
2019-01-25 21:36   ` Paul Moore
2019-01-23 18:35 ` [PATCH ghak100 V2 2/2] audit: ignore fcaps on umount Richard Guy Briggs
2019-01-25 21:45   ` Paul Moore
2019-01-25 22:27     ` Richard Guy Briggs
2019-01-28 23:25       ` Paul Moore
2019-01-31  2:20         ` Paul Moore
2019-02-01 20:42   ` Nathan Chancellor
2019-02-01 21:05     ` Paul Moore [this message]
2019-02-01 21:57       ` Richard Guy Briggs
2019-02-01 22:27         ` Paul Moore
2019-02-01 22:49         ` Richard Guy Briggs

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=CAHC9VhTzy-0cPgD01XyRRPcsgtoV2NZ1NEzun388jPmXmEB-7Q@mail.gmail.com \
    --to=paul@paul-moore.com \
    --cc=eparis@redhat.com \
    --cc=linux-audit@redhat.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=natechancellor@gmail.com \
    --cc=rgb@redhat.com \
    --cc=sgrubb@redhat.com \
    --cc=viro@zeniv.linux.org.uk \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).