From: NeilBrown <neil@xxxxxxxxxx>
The calling convention for nfs4_client_to_reclaim() is clumsy in that
the caller need to free memory if the function fails. It is much
cleaner if the function frees its own memory.
This patch changes nfs4_client_to_reclaim() to re-allocate the .data
fields to be stored in the newly allocated struct nfs4_client_reclaim,
and to free everything on failure.
__cld_pipe_inprogress_downcall() needs to allocate the data anyway to
copy it from user-space, so now that data is allocated twice. I think
that is a small price to pay for a cleaner interface.
Signed-off-by: NeilBrown <neil@xxxxxxxxxx>
---
fs/nfsd/nfs4recover.c | 67 +++++++++++++++----------------------------
fs/nfsd/nfs4state.c | 22 ++++++++++++--
2 files changed, 42 insertions(+), 47 deletions(-)
diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
index 990fe3d958ed..1605538ecb80 100644
--- a/fs/nfsd/nfs4recover.c
+++ b/fs/nfsd/nfs4recover.c
@@ -147,24 +147,13 @@ legacy_recdir_name_error(struct nfs4_client *clp, int error)
static void
__nfsd4_create_reclaim_record_grace(struct nfs4_client *clp,
- const char *dname, int len, struct nfsd_net *nn)
+ char *dname, struct nfsd_net *nn)
{
- struct xdr_netobj name;
+ struct xdr_netobj name = { .len = strlen(dname), .data = dname };
struct xdr_netobj princhash = { .len = 0, .data = NULL };
struct nfs4_client_reclaim *crp;
- name.data = kmemdup(dname, len, GFP_KERNEL);
- if (!name.data) {
- dprintk("%s: failed to allocate memory for name.data!\n",
- __func__);
- return;
- }
- name.len = len;
crp = nfs4_client_to_reclaim(name, princhash, nn);
- if (!crp) {
- kfree(name.data);
- return;
- }
crp->cr_clp = clp;
}
@@ -223,8 +212,7 @@ nfsd4_create_clid_dir(struct nfs4_client *clp)
inode_unlock(d_inode(dir));
if (status == 0) {
if (nn->in_grace)
- __nfsd4_create_reclaim_record_grace(clp, dname,
- HEXDIR_LEN, nn);
+ __nfsd4_create_reclaim_record_grace(clp, dname, nn);
vfs_fsync(nn->rec_file, 0);
} else {
printk(KERN_ERR "NFSD: failed to write recovery record"
@@ -461,7 +449,7 @@ nfsd4_recdir_purge_old(struct nfsd_net *nn)
static int
load_recdir(struct dentry *parent, char *cname, struct nfsd_net *nn)
{
- struct xdr_netobj name;
+ struct xdr_netobj name = { .len = HEXDIR_LEN, .data = cname };
struct xdr_netobj princhash = { .len = 0, .data = NULL };
if (strlen(cname) != HEXDIR_LEN - 1) {
@@ -470,16 +458,7 @@ load_recdir(struct dentry *parent, char *cname, struct nfsd_net *nn)
/* Keep trying; maybe the others are OK: */
return 0;
}
- name.data = kstrdup(cname, GFP_KERNEL);
- if (!name.data) {
- dprintk("%s: failed to allocate memory for name.data!\n",
- __func__);
- goto out;
- }
- name.len = HEXDIR_LEN;
- if (!nfs4_client_to_reclaim(name, princhash, nn))
- kfree(name.data);
-out:
+ nfs4_client_to_reclaim(name, princhash, nn);
return 0;
}
@@ -777,6 +756,8 @@ __cld_pipe_inprogress_downcall(const struct cld_msg_v2 __user *cmsg,
{
uint8_t cmd, princhashlen;
struct xdr_netobj name, princhash = { .len = 0, .data = NULL };
+ char *namecopy __free(kfree) = NULL;
+ char *princhashcopy __free(kfree) = NULL;
uint16_t namelen;
if (get_user(cmd, &cmsg->cm_cmd)) {
@@ -794,19 +775,19 @@ __cld_pipe_inprogress_downcall(const struct cld_msg_v2 __user *cmsg,
dprintk("%s: invalid namelen (%u)", __func__, namelen);
return -EINVAL;
}
- name.data = memdup_user(&ci->cc_name.cn_id, namelen);
- if (IS_ERR(name.data))
- return PTR_ERR(name.data);
+ namecopy = memdup_user(&ci->cc_name.cn_id, namelen);
+ if (IS_ERR(namecopy))
+ return PTR_ERR(namecopy);
+ name.data = namecopy;
name.len = namelen;
get_user(princhashlen, &ci->cc_princhash.cp_len);
if (princhashlen > 0) {
- princhash.data = memdup_user(
- &ci->cc_princhash.cp_data,
- princhashlen);
- if (IS_ERR(princhash.data)) {
- kfree(name.data);
- return PTR_ERR(princhash.data);
- }
+ princhashcopy = memdup_user(
+ &ci->cc_princhash.cp_data,
+ princhashlen);
+ if (IS_ERR(princhashcopy))
+ return PTR_ERR(princhashcopy);
+ princhash.data = princhashcopy;
princhash.len = princhashlen;
} else
princhash.len = 0;
@@ -820,9 +801,10 @@ __cld_pipe_inprogress_downcall(const struct cld_msg_v2 __user *cmsg,
dprintk("%s: invalid namelen (%u)", __func__, namelen);
return -EINVAL;
}
- name.data = memdup_user(&cnm->cn_id, namelen);
- if (IS_ERR(name.data))
- return PTR_ERR(name.data);
+ namecopy = memdup_user(&cnm->cn_id, namelen);
+ if (IS_ERR(namecopy))
+ return PTR_ERR(namecopy);
+ name.data = namecopy;
name.len = namelen;
}
#ifdef CONFIG_NFSD_LEGACY_CLIENT_TRACKING
@@ -830,15 +812,12 @@ __cld_pipe_inprogress_downcall(const struct cld_msg_v2 __user *cmsg,
struct cld_net *cn = nn->cld_net;
name.len = name.len - 5;
- memmove(name.data, name.data + 5, name.len);
+ name.data = name.data + 5;
cn->cn_has_legacy = true;
}
#endif
- if (!nfs4_client_to_reclaim(name, princhash, nn)) {
- kfree(name.data);
- kfree(princhash.data);
+ if (!nfs4_client_to_reclaim(name, princhash, nn))
return -EFAULT;
- }
return nn->client_tracking_ops->msglen;
}
return -EFAULT;
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index e45f67924bd0..fa073353f30b 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -8801,9 +8801,6 @@ nfs4_has_reclaimed_state(struct xdr_netobj name, struct nfsd_net *nn)
/*
* failure => all reset bets are off, nfserr_no_grace...
- *
- * The caller is responsible for freeing name.data if NULL is returned (it
- * will be freed in nfs4_remove_reclaim_record in the normal case).
*/
struct nfs4_client_reclaim *
nfs4_client_to_reclaim(struct xdr_netobj name, struct xdr_netobj princhash,
@@ -8812,6 +8809,22 @@ nfs4_client_to_reclaim(struct xdr_netobj name, struct xdr_netobj princhash,
unsigned int strhashval;
struct nfs4_client_reclaim *crp;
+ name.data = kmemdup(name.data, name.len, GFP_KERNEL);
+ if (!name.data) {
+ dprintk("%s: failed to allocate memory for name.data!\n",
+ __func__);
+ return NULL;
+ }
+ if (princhash.len) {
+ princhash.data = kmemdup(princhash.data, princhash.len, GFP_KERNEL);
+ if (!princhash.data) {
+ dprintk("%s: failed to allocate memory for princhash.data!\n",
+ __func__);
+ kfree(name.data);
+ return NULL;
+ }
+ } else
+ princhash.data = NULL;
crp = alloc_reclaim();
if (crp) {
strhashval = clientstr_hashval(name);
@@ -8823,6 +8836,9 @@ nfs4_client_to_reclaim(struct xdr_netobj name, struct xdr_netobj princhash,
crp->cr_princhash.len = princhash.len;
crp->cr_clp = NULL;
nn->reclaim_str_hashtbl_size++;
+ } else {
+ kfree(name.data);
+ kfree(princhash.data);
}
return crp;
}
--
2.50.0.107.gf914562f5916.dirty
