linux-kernel - Re: [rfc] fcntl: Add F_GETOWNER

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Wed, 28 Mar 2012 19:43:12 +0000
From:	"Serge E. Hallyn" <serge@...lyn.com>
To:	Cyrill Gorcunov <gorcunov@...nvz.org>
Cc:	"Eric W. Biederman" <ebiederm@...ssion.com>,
	"Serge E. Hallyn" <serge@...lyn.com>,
	Oleg Nesterov <oleg@...hat.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Pavel Emelyanov <xemul@...allels.com>,
	"Serge E. Hallyn" <serge.hallyn@...onical.com>
Subject: Re: [rfc] fcntl: Add F_GETOWNER_UIDS option

Quoting Cyrill Gorcunov (gorcunov@...nvz.org):
> Something like below i think (yet untested).
> 
> 	Cyrill
> ---
> From: Cyrill Gorcunov <gorcunov@...nvz.org>
> Subject: fcntl: Add F_GETOWNER_UIDS option v2
> 
> When we restore file descriptors we would like
> them to look exactly as they were at dumping time.
> 
> With help of fcntl it's almost possible, the missing
> snippet is file owners UIDs.
> 
> To be able to read their values the F_GETOWNER_UIDS
> is introduced.
> 
> This option is valid iif CONFIG_CHECKPOINT_RESTORE
> is turned on, otherwise returning -EINVAL.
> 
> Also the call must be done from initial user-namespace
> for a while. This limitation migh be relaxed after, once
> proper [e]uids mapping between namespaces and file owners
> will be implemented.
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov@...nvz.org>
> CC: "Eric W. Biederman" <ebiederm@...ssion.com>
> CC: "Serge E. Hallyn" <serge@...lyn.com>

Sorry to cause trouble, cause as i say it's not particularly important
right now, but note that what you're doing here only ensures that the
person doing the checkpoint is in init_user_ns.  It says nothing about
the credentials in the fown_struct.

If you want to go with this patch, it's fine by me.  If you want to
just add the struct cred to the f_owner and do proper uid conversion,
I'll support that too.  (Just grab a ref to the cred in
fs/fcntl.c:f_modown(), and drop the ref in fs/file_table.c:__fput() ).

Nit: note the indenting in the init_user_ns check below.

> CC: Oleg Nesterov <oleg@...hat.com>
> CC: Andrew Morton <akpm@...ux-foundation.org>
> CC: Pavel Emelyanov <xemul@...allels.com>
> ---
>  fs/fcntl.c                  |   32 ++++++++++++++++++++++++++++++++
>  include/asm-generic/fcntl.h |    4 ++++
>  security/selinux/hooks.c    |    1 +
>  3 files changed, 37 insertions(+)
> 
> Index: linux-2.6.git/fs/fcntl.c
> ===================================================================
> --- linux-2.6.git.orig/fs/fcntl.c
> +++ linux-2.6.git/fs/fcntl.c
> @@ -20,6 +20,7 @@
>  #include <linux/signal.h>
>  #include <linux/rcupdate.h>
>  #include <linux/pid_namespace.h>
> +#include <linux/user_namespace.h>
>  
>  #include <asm/poll.h>
>  #include <asm/siginfo.h>
> @@ -340,6 +341,34 @@ static int f_getown_ex(struct file *filp
>  	return ret;
>  }
>  
> +#ifdef CONFIG_CHECKPOINT_RESTORE
> +static int f_getowner_uids(struct file *filp, unsigned long arg)
> +{
> +	struct user_namespace *user_ns = current_user_ns();
> +	uid_t * __user dst = (void * __user)arg;
> +	uid_t src[2];
> +	int err;
> +
> +        if (user_ns != &init_user_ns)
> +                return -EINVAL;
> +
> +	read_lock(&filp->f_owner.lock);
> +	src[0] = filp->f_owner.uid;
> +	src[1] = filp->f_owner.euid;
> +	read_unlock(&filp->f_owner.lock);
> +
> +	err  = put_user(src[0], &dst[0]);
> +	err |= put_user(src[1], &dst[1]);
> +
> +	return err;
> +}
> +#else
> +static int f_getowner_uids(struct file *filp, unsigned long arg)
> +{
> +	return -EINVAL;
> +}
> +#endif
> +
>  static long do_fcntl(int fd, unsigned int cmd, unsigned long arg,
>  		struct file *filp)
>  {
> @@ -396,6 +425,9 @@ static long do_fcntl(int fd, unsigned in
>  	case F_SETOWN_EX:
>  		err = f_setown_ex(filp, arg);
>  		break;
> +	case F_GETOWNER_UIDS:
> +		err = f_getowner_uids(filp, arg);
> +		break;
>  	case F_GETSIG:
>  		err = filp->f_owner.signum;
>  		break;
> Index: linux-2.6.git/include/asm-generic/fcntl.h
> ===================================================================
> --- linux-2.6.git.orig/include/asm-generic/fcntl.h
> +++ linux-2.6.git/include/asm-generic/fcntl.h
> @@ -120,6 +120,10 @@
>  #define F_GETOWN_EX	16
>  #endif
>  
> +#ifndef F_GETOWNER_UIDS
> +#define F_GETOWNER_UIDS	17
> +#endif
> +
>  #define F_OWNER_TID	0
>  #define F_OWNER_PID	1
>  #define F_OWNER_PGRP	2
> Index: linux-2.6.git/security/selinux/hooks.c
> ===================================================================
> --- linux-2.6.git.orig/security/selinux/hooks.c
> +++ linux-2.6.git/security/selinux/hooks.c
> @@ -3138,6 +3138,7 @@ static int selinux_file_fcntl(struct fil
>  	case F_GETFL:
>  	case F_GETOWN:
>  	case F_GETSIG:
> +	case F_GETOWNER_UIDS:
>  		/* Just check FD__USE permission */
>  		err = file_has_perm(cred, file, 0);
>  		break;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/