NFS: Fix double d_drop in nfs_instantiate() error path
If the LOOKUP or GETATTR in nfs_instantiate fail, nfs_instantiate will do a d_drop before returning. But some callers already do a d_drop in the case of an error return. Make certain we do only one d_drop in all error paths. This issue was introduced because over time, the symlink proc API diverged slightly from the create/mkdir/mknod proc API. To prevent other coding mistakes of this type, change the symlink proc API to be more like create/mkdir/mknod and move the nfs_instantiate call into the symlink proc routines so it is used in exactly the same way for create, mkdir, mknod, and symlink. Test plan: Connectathon, all versions of NFS. Signed-off-by: Chuck Lever <chuck.lever@oracle.com> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
This commit is contained in:
parent
d3db90e270
commit
4f390c152b
16
fs/nfs/dir.c
16
fs/nfs/dir.c
@ -1147,23 +1147,20 @@ int nfs_instantiate(struct dentry *dentry, struct nfs_fh *fhandle,
|
||||
struct inode *dir = dentry->d_parent->d_inode;
|
||||
error = NFS_PROTO(dir)->lookup(dir, &dentry->d_name, fhandle, fattr);
|
||||
if (error)
|
||||
goto out_err;
|
||||
return error;
|
||||
}
|
||||
if (!(fattr->valid & NFS_ATTR_FATTR)) {
|
||||
struct nfs_server *server = NFS_SB(dentry->d_sb);
|
||||
error = server->nfs_client->rpc_ops->getattr(server, fhandle, fattr);
|
||||
if (error < 0)
|
||||
goto out_err;
|
||||
return error;
|
||||
}
|
||||
inode = nfs_fhget(dentry->d_sb, fhandle, fattr);
|
||||
error = PTR_ERR(inode);
|
||||
if (IS_ERR(inode))
|
||||
goto out_err;
|
||||
return error;
|
||||
d_instantiate(dentry, inode);
|
||||
return 0;
|
||||
out_err:
|
||||
d_drop(dentry);
|
||||
return error;
|
||||
}
|
||||
|
||||
/*
|
||||
@ -1448,8 +1445,6 @@ static int
|
||||
nfs_symlink(struct inode *dir, struct dentry *dentry, const char *symname)
|
||||
{
|
||||
struct iattr attr;
|
||||
struct nfs_fattr sym_attr;
|
||||
struct nfs_fh sym_fh;
|
||||
struct qstr qsymname;
|
||||
int error;
|
||||
|
||||
@ -1473,12 +1468,9 @@ dentry->d_parent->d_name.name, dentry->d_name.name);
|
||||
|
||||
lock_kernel();
|
||||
nfs_begin_data_update(dir);
|
||||
error = NFS_PROTO(dir)->symlink(dir, &dentry->d_name, &qsymname,
|
||||
&attr, &sym_fh, &sym_attr);
|
||||
error = NFS_PROTO(dir)->symlink(dir, dentry, &qsymname, &attr);
|
||||
nfs_end_data_update(dir);
|
||||
if (!error)
|
||||
error = nfs_instantiate(dentry, &sym_fh, &sym_attr);
|
||||
else
|
||||
d_drop(dentry);
|
||||
unlock_kernel();
|
||||
return error;
|
||||
|
@ -544,23 +544,23 @@ nfs3_proc_link(struct inode *inode, struct inode *dir, struct qstr *name)
|
||||
}
|
||||
|
||||
static int
|
||||
nfs3_proc_symlink(struct inode *dir, struct qstr *name, struct qstr *path,
|
||||
struct iattr *sattr, struct nfs_fh *fhandle,
|
||||
struct nfs_fattr *fattr)
|
||||
nfs3_proc_symlink(struct inode *dir, struct dentry *dentry, struct qstr *path,
|
||||
struct iattr *sattr)
|
||||
{
|
||||
struct nfs_fattr dir_attr;
|
||||
struct nfs_fh fhandle;
|
||||
struct nfs_fattr fattr, dir_attr;
|
||||
struct nfs3_symlinkargs arg = {
|
||||
.fromfh = NFS_FH(dir),
|
||||
.fromname = name->name,
|
||||
.fromlen = name->len,
|
||||
.fromname = dentry->d_name.name,
|
||||
.fromlen = dentry->d_name.len,
|
||||
.topath = path->name,
|
||||
.tolen = path->len,
|
||||
.sattr = sattr
|
||||
};
|
||||
struct nfs3_diropres res = {
|
||||
.dir_attr = &dir_attr,
|
||||
.fh = fhandle,
|
||||
.fattr = fattr
|
||||
.fh = &fhandle,
|
||||
.fattr = &fattr
|
||||
};
|
||||
struct rpc_message msg = {
|
||||
.rpc_proc = &nfs3_procedures[NFS3PROC_SYMLINK],
|
||||
@ -571,11 +571,17 @@ nfs3_proc_symlink(struct inode *dir, struct qstr *name, struct qstr *path,
|
||||
|
||||
if (path->len > NFS3_MAXPATHLEN)
|
||||
return -ENAMETOOLONG;
|
||||
dprintk("NFS call symlink %s -> %s\n", name->name, path->name);
|
||||
|
||||
dprintk("NFS call symlink %s -> %s\n", dentry->d_name.name,
|
||||
path->name);
|
||||
nfs_fattr_init(&dir_attr);
|
||||
nfs_fattr_init(fattr);
|
||||
nfs_fattr_init(&fattr);
|
||||
status = rpc_call_sync(NFS_CLIENT(dir), &msg, 0);
|
||||
nfs_post_op_update_inode(dir, &dir_attr);
|
||||
if (status != 0)
|
||||
goto out;
|
||||
status = nfs_instantiate(dentry, &fhandle, &fattr);
|
||||
out:
|
||||
dprintk("NFS reply symlink: %d\n", status);
|
||||
return status;
|
||||
}
|
||||
|
@ -2084,24 +2084,24 @@ static int nfs4_proc_link(struct inode *inode, struct inode *dir, struct qstr *n
|
||||
return err;
|
||||
}
|
||||
|
||||
static int _nfs4_proc_symlink(struct inode *dir, struct qstr *name,
|
||||
struct qstr *path, struct iattr *sattr, struct nfs_fh *fhandle,
|
||||
struct nfs_fattr *fattr)
|
||||
static int _nfs4_proc_symlink(struct inode *dir, struct dentry *dentry,
|
||||
struct qstr *path, struct iattr *sattr)
|
||||
{
|
||||
struct nfs_server *server = NFS_SERVER(dir);
|
||||
struct nfs_fattr dir_fattr;
|
||||
struct nfs_fh fhandle;
|
||||
struct nfs_fattr fattr, dir_fattr;
|
||||
struct nfs4_create_arg arg = {
|
||||
.dir_fh = NFS_FH(dir),
|
||||
.server = server,
|
||||
.name = name,
|
||||
.name = &dentry->d_name,
|
||||
.attrs = sattr,
|
||||
.ftype = NF4LNK,
|
||||
.bitmask = server->attr_bitmask,
|
||||
};
|
||||
struct nfs4_create_res res = {
|
||||
.server = server,
|
||||
.fh = fhandle,
|
||||
.fattr = fattr,
|
||||
.fh = &fhandle,
|
||||
.fattr = &fattr,
|
||||
.dir_fattr = &dir_fattr,
|
||||
};
|
||||
struct rpc_message msg = {
|
||||
@ -2113,27 +2113,28 @@ static int _nfs4_proc_symlink(struct inode *dir, struct qstr *name,
|
||||
|
||||
if (path->len > NFS4_MAXPATHLEN)
|
||||
return -ENAMETOOLONG;
|
||||
|
||||
arg.u.symlink = path;
|
||||
nfs_fattr_init(fattr);
|
||||
nfs_fattr_init(&fattr);
|
||||
nfs_fattr_init(&dir_fattr);
|
||||
|
||||
status = rpc_call_sync(NFS_CLIENT(dir), &msg, 0);
|
||||
if (!status)
|
||||
if (!status) {
|
||||
update_changeattr(dir, &res.dir_cinfo);
|
||||
nfs_post_op_update_inode(dir, res.dir_fattr);
|
||||
nfs_post_op_update_inode(dir, res.dir_fattr);
|
||||
status = nfs_instantiate(dentry, &fhandle, &fattr);
|
||||
}
|
||||
return status;
|
||||
}
|
||||
|
||||
static int nfs4_proc_symlink(struct inode *dir, struct qstr *name,
|
||||
struct qstr *path, struct iattr *sattr, struct nfs_fh *fhandle,
|
||||
struct nfs_fattr *fattr)
|
||||
static int nfs4_proc_symlink(struct inode *dir, struct dentry *dentry,
|
||||
struct qstr *path, struct iattr *sattr)
|
||||
{
|
||||
struct nfs4_exception exception = { };
|
||||
int err;
|
||||
do {
|
||||
err = nfs4_handle_exception(NFS_SERVER(dir),
|
||||
_nfs4_proc_symlink(dir, name, path, sattr,
|
||||
fhandle, fattr),
|
||||
_nfs4_proc_symlink(dir, dentry, path, sattr),
|
||||
&exception);
|
||||
} while (exception.retry);
|
||||
return err;
|
||||
|
@ -425,14 +425,15 @@ nfs_proc_link(struct inode *inode, struct inode *dir, struct qstr *name)
|
||||
}
|
||||
|
||||
static int
|
||||
nfs_proc_symlink(struct inode *dir, struct qstr *name, struct qstr *path,
|
||||
struct iattr *sattr, struct nfs_fh *fhandle,
|
||||
struct nfs_fattr *fattr)
|
||||
nfs_proc_symlink(struct inode *dir, struct dentry *dentry, struct qstr *path,
|
||||
struct iattr *sattr)
|
||||
{
|
||||
struct nfs_fh fhandle;
|
||||
struct nfs_fattr fattr;
|
||||
struct nfs_symlinkargs arg = {
|
||||
.fromfh = NFS_FH(dir),
|
||||
.fromname = name->name,
|
||||
.fromlen = name->len,
|
||||
.fromname = dentry->d_name.name,
|
||||
.fromlen = dentry->d_name.len,
|
||||
.topath = path->name,
|
||||
.tolen = path->len,
|
||||
.sattr = sattr
|
||||
@ -445,11 +446,23 @@ nfs_proc_symlink(struct inode *dir, struct qstr *name, struct qstr *path,
|
||||
|
||||
if (path->len > NFS2_MAXPATHLEN)
|
||||
return -ENAMETOOLONG;
|
||||
dprintk("NFS call symlink %s -> %s\n", name->name, path->name);
|
||||
nfs_fattr_init(fattr);
|
||||
fhandle->size = 0;
|
||||
|
||||
dprintk("NFS call symlink %s -> %s\n", dentry->d_name.name,
|
||||
path->name);
|
||||
status = rpc_call_sync(NFS_CLIENT(dir), &msg, 0);
|
||||
nfs_mark_for_revalidate(dir);
|
||||
|
||||
/*
|
||||
* V2 SYMLINK requests don't return any attributes. Setting the
|
||||
* filehandle size to zero indicates to nfs_instantiate that it
|
||||
* should fill in the data with a LOOKUP call on the wire.
|
||||
*/
|
||||
if (status == 0) {
|
||||
nfs_fattr_init(&fattr);
|
||||
fhandle.size = 0;
|
||||
status = nfs_instantiate(dentry, &fhandle, &fattr);
|
||||
}
|
||||
|
||||
dprintk("NFS reply symlink: %d\n", status);
|
||||
return status;
|
||||
}
|
||||
|
@ -793,9 +793,8 @@ struct nfs_rpc_ops {
|
||||
int (*rename) (struct inode *, struct qstr *,
|
||||
struct inode *, struct qstr *);
|
||||
int (*link) (struct inode *, struct inode *, struct qstr *);
|
||||
int (*symlink) (struct inode *, struct qstr *, struct qstr *,
|
||||
struct iattr *, struct nfs_fh *,
|
||||
struct nfs_fattr *);
|
||||
int (*symlink) (struct inode *, struct dentry *, struct qstr *,
|
||||
struct iattr *);
|
||||
int (*mkdir) (struct inode *, struct dentry *, struct iattr *);
|
||||
int (*rmdir) (struct inode *, struct qstr *);
|
||||
int (*readdir) (struct dentry *, struct rpc_cred *,
|
||||
|
Loading…
Reference in New Issue
Block a user