Linux NFS development
 help / color / mirror / Atom feed
From: Jeff Layton <jlayton@kernel.org>
To: Chuck Lever III <chuck.lever@oracle.com>,
	Jason Gunthorpe <jgg@nvidia.com>
Cc: Yishai Hadas <yishaih@nvidia.com>,
	Shachar Kagan <skagan@nvidia.com>,
	Itay Aveksis <itayav@nvidia.com>, Neil Brown <neilb@suse.de>,
	Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH v5 3/5] nfsd: rework refcounting in filecache
Date: Tue, 17 Jan 2023 10:59:02 -0500	[thread overview]
Message-ID: <d69c0c643c23c56408640c4b7d4fc2acac4bc66f.camel@kernel.org> (raw)
In-Reply-To: <9CAD601F-C323-405F-840A-9CBAF520948B@oracle.com>

On Tue, 2023-01-17 at 15:22 +0000, Chuck Lever III wrote:
> 
> > On Jan 17, 2023, at 10:16 AM, Jason Gunthorpe <jgg@nvidia.com> wrote:
> > 
> > On Tue, Nov 01, 2022 at 10:46:45AM -0400, Jeff Layton wrote:
> > > The filecache refcounting is a bit non-standard for something searchable
> > > by RCU, in that we maintain a sentinel reference while it's hashed. This
> > > in turn requires that we have to do things differently in the "put"
> > > depending on whether its hashed, which we believe to have led to races.
> > > 
> > > There are other problems in here too. nfsd_file_close_inode_sync can end
> > > up freeing an nfsd_file while there are still outstanding references to
> > > it, and there are a number of subtle ToC/ToU races.
> > > 
> > > Rework the code so that the refcount is what drives the lifecycle. When
> > > the refcount goes to zero, then unhash and rcu free the object.
> > > 
> > > With this change, the LRU carries a reference. Take special care to
> > > deal with it when removing an entry from the list.
> > > 
> > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > 
> > Our test team is getting crashes that bisection pointed at this
> > patch. It seems like there are multiple parallel crash reports so the
> > whole thing is a mess to read:
> > 
> > [  875.548965] BUG: kernel NULL pointer dereference, address: 00000000000000d0
> > [  875.548968] ------------[ cut here ]------------
> > [  875.548972] refcount_t: underflow; use-after-free.
> > [  875.548992] WARNING: CPU: 4 PID: 12145 at lib/refcount.c:28 refcount_warn_saturate+0xd8/0xe0
> > [  875.549851] #PF: supervisor read access in kernel mode
> > [  875.550158] Modules linked in:
> > [  875.550752] #PF: error_code(0x0000) - not-present page
> > [  875.551269]  nfsd
> > [  875.551878] PGD 0
> > [  875.552069]  iptable_raw
> > [  875.552677] P4D 0
> > [  875.552824]  bonding mlx5_vfio_pci
> > [  875.553095]
> > [  875.553255]  rdma_ucm ipip
> > [  875.553525] Oops: 0000 [#1] SMP
> > [  875.553733]  tunnel4
> > [  875.553941] CPU: 0 PID: 12147 Comm: nfsd Not tainted 6.1.0-rc7_ac3a2585f018 #1
> > [  875.554109]  ip_gre ib_umad
> > [  875.554517] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
> > [  875.554656]  nf_tables vfio_pci
> > [  875.555508] RIP: 0010:vfs_setlease+0x27/0x70
> > [  875.555695]  vfio_pci_core vfio_virqfd
> > [  875.557015] Code: ff ff 90 0f 1f 44 00 00 41 54 49 89 d4 55 48 89 fd 48 83 ec 10 48 85 d2 74 06 48 83 fe 02 75 1f 48 8b 45 28 4c 89 e2 48 89 ef <48> 8b 80 d0 00 00 00 48 85 c0 74 2c 48 83 c4 10 5d 41 5c ff e0 48
> > [  875.557209]  vfio_iommu_type1
> > [  875.557406] RSP: 0018:ffff88810378bdb0 EFLAGS: 00010246
> > [  875.557634]  mlx5_ib
> > [  875.558446]
> > [  875.558628]  vfio
> > [  875.558862] RAX: 0000000000000000 RBX: ffff88824866c000 RCX: ffff88810378bdd8
> > [  875.559006]  ib_uverbs
> > [  875.559092] RDX: 0000000000000000 RSI: 0000000000000002 RDI: ffff88812560a200
> > [  875.559218]  ib_ipoib
> > [  875.559557] RBP: ffff88812560a200 R08: ffff8881da5ecf00 R09: ffffffff824064e0
> > [  875.559704]  mlx5_core
> > [  875.560021] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
> > [  875.560165]  ip6_gre
> > [  875.560488] R13: ffff8881da5ecf00 R14: ffff888110e62028 R15: ffff888110e621a0
> > [  875.560634]  gre
> > [  875.560959] FS:  0000000000000000(0000) GS:ffff88852c800000(0000) knlGS:0000000000000000
> > [  875.561108]  ip6_tunnel
> > [  875.561432] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [  875.561554]  tunnel6
> > [  875.561928] CR2: 00000000000000d0 CR3: 00000001ca27d001 CR4: 0000000000372eb0
> > [  875.562084]  geneve
> > [  875.562349] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > [  875.562493]  nfnetlink_cttimeout
> > [  875.562822] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > [  875.562962]  openvswitch
> > [  875.563292] Call Trace:
> > [  875.563298]  <TASK>
> > [  875.563503]  nsh
> > [  875.563839]  destroy_unhashed_deleg+0x58/0xc0 [nfsd]
> 
> We are aware of this failure mode. Actually this started well before
> that particular commit.
> 
> Our problem has been that no one has been able to provide a reliable
> reproducer, so we can't figure out why it's happening. If you have a
> way to reproduce this failure reliably, can you capture a vmcore or
> enable KASAN and get a little more information?
> 

It's possible that this crash may be related to the problem that was
fixed here:

    commit 0b3a551fa58b4da941efeb209b3770868e2eddd7
    Author: Jeff Layton <jlayton@kernel.org>
    Date:   Thu Jan 5 14:55:56 2023 -0500

        nfsd: fix handling of cached open files in nfsd4_open codepath
    
Unfortunately, that hasn't trickled into v6.1 kernels so far.
-- 
Jeff Layton <jlayton@kernel.org>

  reply	other threads:[~2023-01-17 15:59 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-01 14:46 [PATCH v5 0/5] nfsd: clean up refcounting in the filecache Jeff Layton
2022-11-01 14:46 ` [PATCH v5 1/5] nfsd: remove the pages_flushed statistic from filecache Jeff Layton
2022-11-01 14:46 ` [PATCH v5 2/5] nfsd: reorganize filecache.c Jeff Layton
2022-11-01 20:59   ` NeilBrown
2022-11-01 21:04     ` Jeff Layton
2022-11-01 14:46 ` [PATCH v5 3/5] nfsd: rework refcounting in filecache Jeff Layton
2022-11-01 17:25   ` Chuck Lever III
2022-11-01 18:03     ` Jeff Layton
2022-11-01 18:57       ` Jeff Layton
2022-11-01 19:13         ` Chuck Lever III
2022-11-01 20:21           ` Jeff Layton
2022-11-01 18:15     ` Jeff Layton
2022-11-01 21:23   ` NeilBrown
2022-11-01 21:39     ` Jeff Layton
2022-11-01 21:49       ` Chuck Lever III
2022-11-01 22:04         ` Chuck Lever III
2022-11-01 22:05       ` NeilBrown
2022-11-01 22:42         ` Jeff Layton
2022-11-02 16:58           ` Jeff Layton
2022-11-02 18:07             ` Chuck Lever III
2022-11-02 18:29               ` Chuck Lever III
2022-11-02 18:34               ` Jeff Layton
2023-01-17 15:16   ` Jason Gunthorpe
2023-01-17 15:22     ` Chuck Lever III
2023-01-17 15:59       ` Jeff Layton [this message]
2023-01-18 16:48         ` Shachar Kagan
2023-01-18 17:18           ` Jeff Layton
2023-01-18 18:41             ` Chuck Lever III
2022-11-01 14:46 ` [PATCH v5 4/5] nfsd: close race between unhashing and LRU addition Jeff Layton
2022-11-01 19:45   ` Chuck Lever III
2022-11-01 21:12   ` Chuck Lever III
2022-11-01 14:46 ` [PATCH v5 5/5] nfsd: fix up the filecache laundrette scheduling Jeff Layton

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=d69c0c643c23c56408640c4b7d4fc2acac4bc66f.camel@kernel.org \
    --to=jlayton@kernel.org \
    --cc=chuck.lever@oracle.com \
    --cc=itayav@nvidia.com \
    --cc=jgg@nvidia.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=neilb@suse.de \
    --cc=skagan@nvidia.com \
    --cc=yishaih@nvidia.com \
    /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