locks: let the caller free file_lock on ->setlease failure

The caller allocated it, the caller should free it. The only issue so far is that we could change the flp pointer even on an error return if the fl_change callback failed. But we can simply move the flp assignment after the fl_change invocation, as the callers don't care about the flp return value if the setlease call failed. Signed-off-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

locks: let the caller free file_lock on ->setlease failure
The caller allocated it, the caller should free it. The only issue so far is that we could change the flp pointer even on an error return if the fl_change callback failed. But we can simply move the flp assignment after the fl_change invocation, as the callers don't care about the flp return value if the setlease call failed. Signed-off-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Christoph Hellwig · Linus Torvalds
1 parent 96f9359345
Showing 5 changed files with 13 additions and 17 deletions Side-by-side Diff
fs/cifs/cifsfs.c
fs/gfs2/file.c
fs/locks.c
fs/nfs/file.c
fs/nfsd/nfs4state.c
@@ -625,11 +625,8 @@
 		   knows that the file won't be changed on the server
 		   by anyone else */
 		return generic_setlease(file, arg, lease);
-	else {
-		if (arg != F_UNLCK)
-			locks_free_lock(*lease);
+	else
 		return -EAGAIN;
-	}
 }
  
 struct file_system_type cifs_fs_type = {
@@ -629,8 +629,6 @@
  
 static int gfs2_setlease(struct file *file, long arg, struct file_lock **fl)
 {
-	if (arg != F_UNLCK)
-		locks_free_lock(*fl);
 	return -EINVAL;
 }
  
@@ -1428,8 +1428,9 @@
 		goto out;
  
 	if (my_before != NULL) {
-		*flp = *my_before;
 		error = lease->fl_lmops->fl_change(my_before, arg);
+		if (!error)
+			*flp = *my_before;
 		goto out;
 	}
  
@@ -1444,8 +1445,6 @@
 	return 0;
  
 out:
-	if (arg != F_UNLCK)
-		locks_free_lock(lease);
 	return error;
 }
 EXPORT_SYMBOL(generic_setlease);
@@ -1524,8 +1523,11 @@
 	}
 	lock_flocks();
 	error = __vfs_setlease(filp, arg, &fl);
-	if (error)
-		goto out_unlock;
+	if (error) {
+		unlock_flocks();
+		locks_free_lock(fl);
+		goto out_free_fasync;
+	}
  
 	/*
 	 * fasync_insert_entry() returns the old entry if any.
  
  
@@ -1541,12 +1543,12 @@
 		fl->fl_type = F_UNLCK | F_INPROGRESS;
 		fl->fl_break_time = jiffies - 10;
 		time_out_leases(inode);
-		goto out_unlock;
+	} else {
+		error = __f_setown(filp, task_pid(current), PIDTYPE_PID, 0);
 	}
-
-	error = __f_setown(filp, task_pid(current), PIDTYPE_PID, 0);
-out_unlock:
 	unlock_flocks();
+
+out_free_fasync:
 	if (new)
 		fasync_free(new);
 	return error;
@@ -884,8 +884,6 @@
 	dprintk("NFS: setlease(%s/%s, arg=%ld)\n",
 			file->f_path.dentry->d_parent->d_name.name,
 			file->f_path.dentry->d_name.name, arg);
-	if (arg != F_UNLCK)
-		locks_free_lock(*fl);
 	return -EINVAL;
 }
@@ -2652,6 +2652,7 @@
 	if ((status = vfs_setlease(fl->fl_file, fl->fl_type, &fl))) {
 		dprintk("NFSD: setlease failed [%d], no delegation\n", status);
 		dp->dl_flock = NULL;
+		locks_free_lock(fl);
 		unhash_delegation(dp);
 		flag = NFS4_OPEN_DELEGATE_NONE;
 		goto out;
...	...	@@ -625,11 +625,8 @@
625	625	knows that the file won't be changed on the server
626	626	by anyone else */
627	627	return generic_setlease(file, arg, lease);
628		- else {
629		- if (arg != F_UNLCK)
630		- locks_free_lock(*lease);
	628	+ else
631	629	return -EAGAIN;
632		- }
633	630	}
634	631
635	632	struct file_system_type cifs_fs_type = {
...	...	@@ -629,8 +629,6 @@
629	629
630	630	static int gfs2_setlease(struct file file, long arg, struct file_lock *fl)
631	631	{
632		- if (arg != F_UNLCK)
633		- locks_free_lock(*fl);
634	632	return -EINVAL;
635	633	}
636	634
...	...	@@ -1428,8 +1428,9 @@
1428	1428	goto out;
1429	1429
1430	1430	if (my_before != NULL) {
1431		- flp = my_before;
1432	1431	error = lease->fl_lmops->fl_change(my_before, arg);
	1432	+ if (!error)
	1433	+ flp = my_before;
1433	1434	goto out;
1434	1435	}
1435	1436
...	...	@@ -1444,8 +1445,6 @@
1444	1445	return 0;
1445	1446
1446	1447	out:
1447		- if (arg != F_UNLCK)
1448		- locks_free_lock(lease);
1449	1448	return error;
1450	1449	}
1451	1450	EXPORT_SYMBOL(generic_setlease);
...	...	@@ -1524,8 +1523,11 @@
1524	1523	}
1525	1524	lock_flocks();
1526	1525	error = __vfs_setlease(filp, arg, &fl);
1527		- if (error)
1528		- goto out_unlock;
	1526	+ if (error) {
	1527	+ unlock_flocks();
	1528	+ locks_free_lock(fl);
	1529	+ goto out_free_fasync;
	1530	+ }
1529	1531
1530	1532	/*
1531	1533	* fasync_insert_entry() returns the old entry if any.
1532	1534
1533	1535
...	...	@@ -1541,12 +1543,12 @@
1541	1543	fl->fl_type = F_UNLCK \| F_INPROGRESS;
1542	1544	fl->fl_break_time = jiffies - 10;
1543	1545	time_out_leases(inode);
1544		- goto out_unlock;
	1546	+ } else {
	1547	+ error = __f_setown(filp, task_pid(current), PIDTYPE_PID, 0);
1545	1548	}
1546		-
1547		- error = __f_setown(filp, task_pid(current), PIDTYPE_PID, 0);
1548		-out_unlock:
1549	1549	unlock_flocks();
	1550	+
	1551	+out_free_fasync:
1550	1552	if (new)
1551	1553	fasync_free(new);
1552	1554	return error;
...	...	@@ -884,8 +884,6 @@
884	884	dprintk("NFS: setlease(%s/%s, arg=%ld)\n",
885	885	file->f_path.dentry->d_parent->d_name.name,
886	886	file->f_path.dentry->d_name.name, arg);
887		- if (arg != F_UNLCK)
888		- locks_free_lock(*fl);
889	887	return -EINVAL;
890	888	}
...	...	@@ -2652,6 +2652,7 @@
2652	2652	if ((status = vfs_setlease(fl->fl_file, fl->fl_type, &fl))) {
2653	2653	dprintk("NFSD: setlease failed [%d], no delegation\n", status);
2654	2654	dp->dl_flock = NULL;
	2655	+ locks_free_lock(fl);
2655	2656	unhash_delegation(dp);
2656	2657	flag = NFS4_OPEN_DELEGATE_NONE;
2657	2658	goto out;