2014-09-11 06:19:49

by NeilBrown

[permalink] [raw]
Subject: [PATCH] NFS: remove BUG possibility in nfs4_open_and_get_state



commit 4fa2c54b5198d09607a534e2fd436581064587ed
NFS: nfs4_do_open should add negative results to the dcache.

used "d_drop(); d_add();" to ensure that a dentry was hashed
as a negative cached entry.
This is not safe if the dentry has an non-NULL ->d_inode.
It will trigger a BUG_ON in d_instantiate().
In that case, d_delete() is needed.

Also, only d_add if the dentry is currently unhashed, it seems
pointless removed and re-adding it unchanged.

Reported-by: Christoph Hellwig <[email protected]>
Fixes: 4fa2c54b5198d09607a534e2fd436581064587ed
Cc: Jeff Layton <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: NeilBrown <[email protected]>

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 7dd8aca31c29..ac2dd953fc18 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2226,9 +2226,13 @@ static int _nfs4_open_and_get_state(struct nfs4_opendata *opendata,
ret = _nfs4_proc_open(opendata);
if (ret != 0) {
if (ret == -ENOENT) {
- d_drop(opendata->dentry);
- d_add(opendata->dentry, NULL);
- nfs_set_verifier(opendata->dentry,
+ dentry = opendata->dentry;
+ if (dentry->d_inode)
+ d_delete(dentry);
+ else if (d_unhashed(dentry))
+ d_add(dentry, NULL);
+
+ nfs_set_verifier(dentry,
nfs_save_change_attribute(opendata->dir->d_inode));
}
goto out;


Attachments:
signature.asc (828.00 B)

2014-09-11 11:15:25

by Jeff Layton

[permalink] [raw]
Subject: Re: [PATCH] NFS: remove BUG possibility in nfs4_open_and_get_state

On Thu, 11 Sep 2014 16:19:37 +1000
NeilBrown <[email protected]> wrote:

>
>
> commit 4fa2c54b5198d09607a534e2fd436581064587ed
> NFS: nfs4_do_open should add negative results to the dcache.
>
> used "d_drop(); d_add();" to ensure that a dentry was hashed
> as a negative cached entry.
> This is not safe if the dentry has an non-NULL ->d_inode.
> It will trigger a BUG_ON in d_instantiate().
> In that case, d_delete() is needed.
>
> Also, only d_add if the dentry is currently unhashed, it seems
> pointless removed and re-adding it unchanged.
>
> Reported-by: Christoph Hellwig <[email protected]>
> Fixes: 4fa2c54b5198d09607a534e2fd436581064587ed
> Cc: Jeff Layton <[email protected]>
> Link: http://lkml.kernel.org/r/[email protected]
> Signed-off-by: NeilBrown <[email protected]>
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 7dd8aca31c29..ac2dd953fc18 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -2226,9 +2226,13 @@ static int _nfs4_open_and_get_state(struct nfs4_opendata *opendata,
> ret = _nfs4_proc_open(opendata);
> if (ret != 0) {
> if (ret == -ENOENT) {
> - d_drop(opendata->dentry);
> - d_add(opendata->dentry, NULL);
> - nfs_set_verifier(opendata->dentry,
> + dentry = opendata->dentry;
> + if (dentry->d_inode)
> + d_delete(dentry);
> + else if (d_unhashed(dentry))
> + d_add(dentry, NULL);
> +
> + nfs_set_verifier(dentry,
> nfs_save_change_attribute(opendata->dir->d_inode));
> }
> goto out;

Acked-by: Jeff Layton <[email protected]>


Attachments:
signature.asc (819.00 B)