Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
From:   "NeilBrown" <neilb@suse.de>
To:     "J. R. Okajima" <hooanon05g@gmail.com>
Cc:     trond.myklebust@hammerspace.com, linux-nfs@vger.kernel.org
Subject: Re: NFS, two d_delete() calls in nfs_unlink()
In-reply-to: <7634.1660728564@jrobl>
References: <7634.1660728564@jrobl>
Date:   Thu, 18 Aug 2022 12:55:31 +1000
Message-id: <166079133167.5425.16635199337074058478@noble.neil.brown.name>
Precedence: bulk

On Wed, 17 Aug 2022, J. R. Okajima wrote:
> Hello NeilBrown and Trond Myklebust,
> 
> By the commin in v6.0-rc1,
> 	3c59366c207e 2022-08-08 NFS: don't unhash dentry during unlink/rename
> nfs_unlink() stopped calling __d_drop().
> And it MAY cause two d_delete() calls. If it happens, the second call
> leads to NULL pointer access because d_inode is already cleared.
> 
> Here is the detail.
> 
> nfs_unlink()
> + nfs_safe_remove()
>   + NFS_PROTO(dir)->remove() <-- returns ENOENT
>   + nfs_dentry_handle_enoent()
>     + if (simple_positive()) d_delete() <-- 1st call and d_inode is cleared
> + nfs_dentry_remove_handle_error()
>   + if (ENOENT) d_delete() <-- second call and NULL d_inode is accessed
> 
> How about adding a condition for d_delete() call in
> nfs_dentry_remove_handle_error(), such like simple_positive()?
> 

Thanks for the report.
This possibility of calling d_delete() twice has been present
since  9019fb391de0 in v5.16.
It is possible that my patch made it more likely or more problematic,
though I cannot see why.
I posted a patch which Trond has applied to his linux-next branch.
It is in linux-next as commit 9a31abb1c009c40

How did you discover this bug, and why do you think my patch
caused it?

Thanks,
NeilBrown