Linux-NFS Archive on lore.kernel.org
 help / color / Atom feed
From: Benjamin Coddington <bcodding@redhat.com>
To: Trond Myklebust <trondmy@gmail.com>,
	Anna Schumaker <Anna.Schumaker@netapp.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>, linux-nfs@vger.kernel.org
Subject: [PATCH 0/3] nfs_instantiate() might succeed leaving dentry negative unhashed
Date: Fri, 13 Sep 2019 08:29:01 -0400
Message-ID: <cover.1568377101.git.bcodding@redhat.com> (raw)

After adding commit b0c6108ecf64 ("nfs_instantiate(): prevent multiple
aliases for directory inode") my NFS client crashes while doing lustre race
tests simultaneously on a local filesystem and the same filesystem exported
via knfsd:

    BUG: unable to handle kernel NULL pointer dereference at 0000000000000028
     Call Trace:
      ? iput+0x76/0x200
      ? d_splice_alias+0x307/0x3c0
      ? dput.part.31+0x96/0x110
      ? nfs_instantiate+0x45/0x160 [nfs]
      nfs3_proc_setacls+0xa/0x20 [nfsv3]
      nfs3_proc_create+0x1cc/0x230 [nfsv3]
      nfs_create+0x83/0x160 [nfs]
      path_openat+0x11aa/0x14d0
      do_filp_open+0x93/0x100
      ? __check_object_size+0xa3/0x181
      do_sys_open+0x184/0x220
      do_syscall_64+0x5b/0x1b0
      entry_SYSCALL_64_after_hwframe+0x65/0xca

   158 static int __nfs3_proc_setacls(struct inode *inode, struct posix_acl *acl,
   159         struct posix_acl *dfacl)
   160 {
161     struct nfs_server *server = NFS_SERVER(inode);

The 0x28 offset is i_sb in struct inode, we passed a NULL inode to
nfs3_proc_setacls().

After taking this apart, I find the dentry in R12 has a NULL inode after
nfs_instantiate(), which makes sense if we move it to the alias just after
nfs_fhget() (See the referenced commit above).  Indeed, on the list of
children is the identical positive dentry that is left behind after
d_splice_alias().  Moving it would usualy be fine for callers, except for
NFSv3 because we want the inode pointer to ride the dentry back up the
stack so we can set ACLs on it and/or set attributes in the case of EXCLUSIVE.

The first patch splits up nfs_instantiate() so that we can have two call
paths, the original - whose callers don't care about what dentry ends up
hashed, and a new one that returns the dentry or the alias.

The second patch modifies NFSv3 to use the latter path for callers that need
to reference the dentry.

The third patch removes a test for positive dentry that seems to be
impossible - I can't find a path for it, and it has never been hit in all my
testing.

Benjamin Coddington (3):
  NFS: Refactor nfs_instantiate() for dentry referencing callers
  NFSv3: use nfs_add_or_obtain() to create and reference inodes
  NFS: remove unused check for negative dentry

 fs/nfs/dir.c           | 41 +++++++++++++++++++++++---------------
 fs/nfs/nfs3proc.c      | 45 +++++++++++++++++++++++++++++++++---------
 include/linux/nfs_fs.h |  3 +++
 3 files changed, 64 insertions(+), 25 deletions(-)

-- 
2.20.1


             reply index

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-13 12:29 Benjamin Coddington [this message]
2019-09-13 12:29 ` [PATCH 1/3] NFS: Refactor nfs_instantiate() for dentry referencing callers Benjamin Coddington
2019-09-13 12:29 ` [PATCH 2/3] NFSv3: use nfs_add_or_obtain() to create and reference inodes Benjamin Coddington
2019-09-13 13:02   ` Benjamin Coddington
2019-09-13 12:29 ` [PATCH 3/3] NFS: remove unused check for negative dentry Benjamin Coddington

Reply instructions:

You may reply publically 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=cover.1568377101.git.bcodding@redhat.com \
    --to=bcodding@redhat.com \
    --cc=Anna.Schumaker@netapp.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=trondmy@gmail.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

Linux-NFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-nfs/0 linux-nfs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-nfs linux-nfs/ https://lore.kernel.org/linux-nfs \
		linux-nfs@vger.kernel.org linux-nfs@archiver.kernel.org
	public-inbox-index linux-nfs


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-nfs


AGPL code for this site: git clone https://public-inbox.org/ public-inbox