Skip to contentSkip to conversation

fs/fcntl: return -ESRCH in f_setown when pid/pgid can't be found

PATCH In review Jeff Layton 2 participants 2 messages Jun 14, 2017 – Jun 15, 2017

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 fs/fcntl: return -ESRCH in f_setown when pid/pgid can't be found

The current implementation of F_SETOWN doesn't properly vet the argument passed in. It never returns an error. If the argument doesn't specify a valid pid/pgid, then we just end up cleaning out the file->f_owner structure.

What we really want is to only clean that out only in the case where userland passed in an argument of 0. For anything else, we want to return ESRCH if it doesn't refer to a valid pid.

The relevant POSIX spec page is here:

    http://pubs.opengroup.org/onlinepubs/9699919799/functions/fcntl.html
Cc Jiri Slaby <[email protected]>
Cc zhong jiang <[email protected]>
---
 fs/fcntl.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)
fs/fcntl.c +13 -5
index 693322e28751..afed3b364979 100644
@@ -112,8 +112,9 @@ EXPORT_SYMBOL(__f_setown);
 int f_setown(struct file *filp, unsigned long arg, int force)
 {
 	enum pid_type type;
-	struct pid *pid;
-	int who = arg;
+	struct pid *pid = NULL;
+	int who = arg, ret = 0;
+
 	type = PIDTYPE_PID;
 	if (who < 0) {
 		/* avoid overflow below */
@@ -123,12 +124,19 @@ int f_setown(struct file *filp, unsigned long arg, int force)
 		type = PIDTYPE_PGID;
 		who = -who;
 	}
+
 	rcu_read_lock();
-	pid = find_vpid(who);
-	__f_setown(filp, pid, type, force);
+	if (who) {
+		pid = find_vpid(who);
+		if (!pid)
+			ret = -ESRCH;
+	}
+
+	if (!ret)
+		__f_setown(filp, pid, type, force);
 	rcu_read_unlock();
 
-	return 0;
+	return ret;
 }
 EXPORT_SYMBOL(f_setown);
 
Signature · 1 line
2.13.0
#1 lore raw

yes, look good to me.

but I found the another issue. if the pass argument is -1. by the spec describe, type should be assigned to PIDTYPE_MAX, Do you think that it deserve another patch ?

Thanks
zhongjiang

Quoted 59 lines · Jeff Layton

The current implementation of F_SETOWN doesn't properly vet the argument passed in. It never returns an error. If the argument doesn't specify a valid pid/pgid, then we just end up cleaning out the file->f_owner structure.

What we really want is to only clean that out only in the case where userland passed in an argument of 0. For anything else, we want to return ESRCH if it doesn't refer to a valid pid.

The relevant POSIX spec page is here:

    http://pubs.opengroup.org/onlinepubs/9699919799/functions/fcntl.html
Cc Jiri Slaby <[email protected]>
Cc zhong jiang <[email protected]>
---
 fs/fcntl.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)
fs/fcntl.c +13 -5
index 693322e28751..afed3b364979 100644
@@ -112,8 +112,9 @@ EXPORT_SYMBOL(__f_setown);
 int f_setown(struct file *filp, unsigned long arg, int force)
 {
 	enum pid_type type;
-	struct pid *pid;
-	int who = arg;
+	struct pid *pid = NULL;
+	int who = arg, ret = 0;
+
 	type = PIDTYPE_PID;
 	if (who < 0) {
 		/* avoid overflow below */
@@ -123,12 +124,19 @@ int f_setown(struct file *filp, unsigned long arg, int force)
 		type = PIDTYPE_PGID;
 		who = -who;
 	}
+
 	rcu_read_lock();
-	pid = find_vpid(who);
-	__f_setown(filp, pid, type, force);
+	if (who) {
+		pid = find_vpid(who);
+		if (!pid)
+			ret = -ESRCH;
+	}
+
+	if (!ret)
+		__f_setown(filp, pid, type, force);
 	rcu_read_unlock();
 
-	return 0;
+	return ret;
 }
 EXPORT_SYMBOL(f_setown);