Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1665902 > unrolled thread

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

Started byJeff Layton <jlayton@redhat.com>
First post2017-06-14 17:00 +0200
Last post2017-06-15 04:00 +0200
Articles 2 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH] fs/fcntl: return -ESRCH in f_setown when pid/pgid can't be found Jeff Layton <jlayton@redhat.com> - 2017-06-14 17:00 +0200
    Re: [PATCH] fs/fcntl: return -ESRCH in f_setown when pid/pgid can't  be found zhong jiang <zhongjiang@huawei.com> - 2017-06-15 04:00 +0200

#1665902 — [PATCH] fs/fcntl: return -ESRCH in f_setown when pid/pgid can't be found

FromJeff Layton <jlayton@redhat.com>
Date2017-06-14 17:00 +0200
Subject[PATCH] fs/fcntl: return -ESRCH in f_setown when pid/pgid can't be found
Message-ID<tShW1-6u5-7@gated-at.bofh.it>
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 <jslaby@suse.cz>
Cc: zhong jiang <zhongjiang@huawei.com>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/fcntl.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/fs/fcntl.c b/fs/fcntl.c
index 693322e28751..afed3b364979 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -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);
 
-- 
2.13.0

[toc] | [next] | [standalone]


#1666366 — Re: [PATCH] fs/fcntl: return -ESRCH in f_setown when pid/pgid can't be found

Fromzhong jiang <zhongjiang@huawei.com>
Date2017-06-15 04:00 +0200
SubjectRe: [PATCH] fs/fcntl: return -ESRCH in f_setown when pid/pgid can't be found
Message-ID<tSseJ-4tL-3@gated-at.bofh.it>
In reply to#1665902
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

On 2017/6/14 22:52, Jeff Layton wrote:
> 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 <jslaby@suse.cz>
> Cc: zhong jiang <zhongjiang@huawei.com>
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
>  fs/fcntl.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/fs/fcntl.c b/fs/fcntl.c
> index 693322e28751..afed3b364979 100644
> --- a/fs/fcntl.c
> +++ b/fs/fcntl.c
> @@ -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);
>  

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web