Skip to content

Commit 6ba434c

Browse files
jtlaytonchucklever
authored andcommitted
nfsd: clean up potential nfsd_file refcount leaks in COPY codepath
There are two different flavors of the nfsd4_copy struct. One is embedded in the compound and is used directly in synchronous copies. The other is dynamically allocated, refcounted and tracked in the client struture. For the embedded one, the cleanup just involves releasing any nfsd_files held on its behalf. For the async one, the cleanup is a bit more involved, and we need to dequeue it from lists, unhash it, etc. There is at least one potential refcount leak in this code now. If the kthread_create call fails, then both the src and dst nfsd_files in the original nfsd4_copy object are leaked. The cleanup in this codepath is also sort of weird. In the async copy case, we'll have up to four nfsd_file references (src and dst for both flavors of copy structure). They are both put at the end of nfsd4_do_async_copy, even though the ones held on behalf of the embedded one outlive that structure. Change it so that we always clean up the nfsd_file refs held by the embedded copy structure before nfsd4_copy returns. Rework cleanup_async_copy to handle both inter and intra copies. Eliminate nfsd4_cleanup_intra_ssc since it now becomes a no-op. Signed-off-by: Jeff Layton <[email protected]> Signed-off-by: Chuck Lever <[email protected]>
1 parent 1f0001d commit 6ba434c

File tree

1 file changed

+10
-13
lines changed

1 file changed

+10
-13
lines changed

fs/nfsd/nfs4proc.c

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1512,7 +1512,6 @@ nfsd4_cleanup_inter_ssc(struct nfsd4_ssc_umount_item *nsui, struct file *filp,
15121512
long timeout = msecs_to_jiffies(nfsd4_ssc_umount_timeout);
15131513

15141514
nfs42_ssc_close(filp);
1515-
nfsd_file_put(dst);
15161515
fput(filp);
15171516

15181517
spin_lock(&nn->nfsd_ssc_lock);
@@ -1562,13 +1561,6 @@ nfsd4_setup_intra_ssc(struct svc_rqst *rqstp,
15621561
&copy->nf_dst);
15631562
}
15641563

1565-
static void
1566-
nfsd4_cleanup_intra_ssc(struct nfsd_file *src, struct nfsd_file *dst)
1567-
{
1568-
nfsd_file_put(src);
1569-
nfsd_file_put(dst);
1570-
}
1571-
15721564
static void nfsd4_cb_offload_release(struct nfsd4_callback *cb)
15731565
{
15741566
struct nfsd4_cb_offload *cbo =
@@ -1683,12 +1675,18 @@ static void dup_copy_fields(struct nfsd4_copy *src, struct nfsd4_copy *dst)
16831675
dst->ss_nsui = src->ss_nsui;
16841676
}
16851677

1678+
static void release_copy_files(struct nfsd4_copy *copy)
1679+
{
1680+
if (copy->nf_src)
1681+
nfsd_file_put(copy->nf_src);
1682+
if (copy->nf_dst)
1683+
nfsd_file_put(copy->nf_dst);
1684+
}
1685+
16861686
static void cleanup_async_copy(struct nfsd4_copy *copy)
16871687
{
16881688
nfs4_free_copy_state(copy);
1689-
nfsd_file_put(copy->nf_dst);
1690-
if (!nfsd4_ssc_is_inter(copy))
1691-
nfsd_file_put(copy->nf_src);
1689+
release_copy_files(copy);
16921690
spin_lock(&copy->cp_clp->async_lock);
16931691
list_del(&copy->copies);
16941692
spin_unlock(&copy->cp_clp->async_lock);
@@ -1748,7 +1746,6 @@ static int nfsd4_do_async_copy(void *data)
17481746
} else {
17491747
nfserr = nfsd4_do_copy(copy, copy->nf_src->nf_file,
17501748
copy->nf_dst->nf_file, false);
1751-
nfsd4_cleanup_intra_ssc(copy->nf_src, copy->nf_dst);
17521749
}
17531750

17541751
do_callback:
@@ -1811,9 +1808,9 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
18111808
} else {
18121809
status = nfsd4_do_copy(copy, copy->nf_src->nf_file,
18131810
copy->nf_dst->nf_file, true);
1814-
nfsd4_cleanup_intra_ssc(copy->nf_src, copy->nf_dst);
18151811
}
18161812
out:
1813+
release_copy_files(copy);
18171814
return status;
18181815
out_err:
18191816
if (async_copy)

0 commit comments

Comments
 (0)