Re: [PATCH] fcntl: Add F_GETOWNER_UIDS option v3

From: Eric W. Biederman
Date: Tue Jun 05 2012 - 12:40:31 EST


Cyrill Gorcunov <gorcunov@xxxxxxxxxx> writes:

> On Tue, Jun 05, 2012 at 09:14:58AM -0700, Eric W. Biederman wrote:
>> Cyrill Gorcunov <gorcunov@xxxxxxxxxx> writes:
>>
>> > 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.
>>
>> You want to use from_kuid_munged instead of from_kuid as you are going
>> directly to userspace, and to userspace for an unmapped uid we want
>> to say 65534 aka nobody instead of -1.
>>
>> > v3:
>> > - rebased on Eric's kuids
>>
>> To be clear this is based on my patchset that has been merged into
>> v3.5-rc1.
>
> Yeah, thanks Eric. Sure I must use _munged version here.
> Updated version below. Thanks!

I don't have any strong opinions about the functionality and
I don't see any bugs so:

Acked-by: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx>

> ---
> From: Cyrill Gorcunov <gorcunov@xxxxxxxxxx>
> Subject: fcntl: Add F_GETOWNER_UIDS option v4
>
> 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.
>
> v4:
> - rebased to use Eric's kuid_ patchset that has been
> merged into v3.5-rc1.
>
> Signed-off-by: Cyrill Gorcunov <gorcunov@xxxxxxxxxx>
> CC: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx>
> CC: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> CC: "Serge E. Hallyn" <serge@xxxxxxxxxx>
> CC: Oleg Nesterov <oleg@xxxxxxxxxx>
> CC: Pavel Emelyanov <xemul@xxxxxxxxxxxxx>
> ---
> fs/fcntl.c | 29 +++++++++++++++++++++++++++++
> include/asm-generic/fcntl.h | 4 ++++
> security/selinux/hooks.c | 1 +
> 3 files changed, 34 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,31 @@ 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;
> +
> + read_lock(&filp->f_owner.lock);
> + src[0] = from_kuid_munged(user_ns, filp->f_owner.uid);
> + src[1] = from_kuid_munged(user_ns, 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 +422,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
> @@ -3181,6 +3181,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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/