Skip to contentSkip to conversation

fcntl: Add F_GETOWNER_UIDS option v3

PATCH In review Cyrill Gorcunov 3 participants 7 messages Jun 5, 2012 – Jun 6, 2012

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

PATCH 1/1 fcntl: Add F_GETOWNER_UIDS option v3

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.

v3:
- rebased on Eric's kuids

CC "Eric W. Biederman" <[email protected]>
CC Andrew Morton <[email protected]>
CC "Serge E. Hallyn" <[email protected]>
CC Oleg Nesterov <[email protected]>
CC Pavel Emelyanov <[email protected]>
---
 fs/fcntl.c                  |   29 +++++++++++++++++++++++++++++
 include/asm-generic/fcntl.h |    4 ++++
 security/selinux/hooks.c    |    1 +
 3 files changed, 34 insertions(+)
fs/fcntl.c +29 -0
Index: 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(user_ns, filp->f_owner.uid);
+	src[1] = from_kuid(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;
include/asm-generic/fcntl.h +4 -0
Index: 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
security/selinux/hooks.c +1 -0
Index: 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;
#1 lore raw
Quoted 11 lines · Cyrill Gorcunov

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.

Quoted 2 lines

v3:
- rebased on Eric's kuids

To be clear this is based on my patchset that has been merged into v3.5-rc1.

Eric

Quoted 93 lines
CC "Eric W. Biederman" <[email protected]>
CC Andrew Morton <[email protected]>
CC "Serge E. Hallyn" <[email protected]>
CC Oleg Nesterov <[email protected]>
CC Pavel Emelyanov <[email protected]>
---
 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
===================================================================

fs/fcntl.c +29 -0
@@ -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(user_ns, filp->f_owner.uid);
+	src[1] = from_kuid(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
===================================================================

include/asm-generic/fcntl.h +4 -0
@@ -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
===================================================================

security/selinux/hooks.c +1 -0
@@ -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;
#2 lore raw
Quoted 18 lines · Eric W. Biederman
Quoted 11 lines · Cyrill Gorcunov

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.

Quoted 5 lines
Quoted 2 lines

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!
---
From: Cyrill Gorcunov <[email protected]>
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.

CC "Eric W. Biederman" <[email protected]>
CC Andrew Morton <[email protected]>
CC "Serge E. Hallyn" <[email protected]>
CC Oleg Nesterov <[email protected]>
CC Pavel Emelyanov <[email protected]>
---
 fs/fcntl.c                  |   29 +++++++++++++++++++++++++++++
 include/asm-generic/fcntl.h |    4 ++++
 security/selinux/hooks.c    |    1 +
 3 files changed, 34 insertions(+)
fs/fcntl.c +29 -0
Index: 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;
include/asm-generic/fcntl.h +4 -0
Index: 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
security/selinux/hooks.c +1 -0
Index: 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;
#3 lore raw
Quoted 19 lines · Cyrill Gorcunov
Quoted 18 lines · Eric W. Biederman
Quoted 11 lines · Cyrill Gorcunov

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.

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

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:

Quoted 113 lines

---
From: Cyrill Gorcunov <[email protected]>
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.

CC "Eric W. Biederman" <[email protected]>
CC Andrew Morton <[email protected]>
CC "Serge E. Hallyn" <[email protected]>
CC Oleg Nesterov <[email protected]>
CC Pavel Emelyanov <[email protected]>
---
 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
===================================================================

fs/fcntl.c +29 -0
@@ -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
===================================================================

include/asm-generic/fcntl.h +4 -0
@@ -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
===================================================================

security/selinux/hooks.c +1 -0
@@ -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;
#4 lore raw
In reply to Cyrill Gorcunov
Quoted 11 lines · On Tue, 5 Jun 2012 12:25:12 +0400 Cyrill Gorcunov <[email protected]> wrote:

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.

um, OK.

But we still have

c-r-prctl-add-ability-to-set-new-mm_struct-exe_file-update-after-mm-num_exe_file_vmas-removal.patch c-r-prctl-add-ability-to-set-new-mm_struct-exe_file-add-minimal-address-test-to-pr_set_mm.patch c-r-prctl-add-ability-to-get-clear_tid_address.patch c-r-prctl-drop-vma-flags-test-on-pr_set_mm_-stack-data-assignment.patch

floating about unmerged due to various unresolved issues. Can we please get all that stuff nailed down before working on new things?

#5 lore raw
Quoted 11 lines · Andrew Morton

um, OK.

But we still have

c-r-prctl-add-ability-to-set-new-mm_struct-exe_file-update-after-mm-num_exe_file_vmas-removal.patch c-r-prctl-add-ability-to-set-new-mm_struct-exe_file-add-minimal-address-test-to-pr_set_mm.patch c-r-prctl-add-ability-to-get-clear_tid_address.patch c-r-prctl-drop-vma-flags-test-on-pr_set_mm_-stack-data-assignment.patch

floating about unmerged due to various unresolved issues. Can we please get all that stuff nailed down before working on new things?

Andrew I understand the frustration but in fairness this patch is just
as old as the patches you are talking about. Cyrill just finished
resolving the unresolved isssues with this one, aka a conflict with user namespace support.

Now I do agree that we need to finish the rest of these things.

Eric

#6 lore raw
In reply to Andrew Morton
Quoted 8 lines · Andrew Morton
Quoted 3 lines

This option is valid iif CONFIG_CHECKPOINT_RESTORE is turned on, otherwise returning -EINVAL.

um, OK.

But we still have

Quoted 7 lines

c-r-prctl-add-ability-to-set-new-mm_struct-exe_file-update-after-mm-num_exe_file_vmas-removal.patch c-r-prctl-add-ability-to-set-new-mm_struct-exe_file-add-minimal-address-test-to-pr_set_mm.patch c-r-prctl-add-ability-to-get-clear_tid_address.patch c-r-prctl-drop-vma-flags-test-on-pr_set_mm_-stack-data-assignment.patch

floating about unmerged due to various unresolved issues. Can we please get all that stuff nailed down before working on new things?

Hmm, Andrew, I must admit I don't understand which unresolved issues here with the rest of prctl SET_MM code. Could you please clarify? Do they not apply on current linux-next, or something like that?

Cyrill