allow the creation of architecture emulation containers where the emulator binary is outside the container

PATCH Superseded James Bottomley 7 participants 14 messages Feb 25, 2016 – Mar 8, 2016 3 messages not in the archive

Subsystems (3)

Documentation VFS exec and binfmt

lore

Share & tools

lore

James Bottomley

author

2016-02-25 19:34 UTC #0 lore raw

v2 cover allow the creation of architecture emulation containers where the emulator binary is outside the container

Changes since v2:

* Use oldfile f_creds for the vfs_open credentials in filp_clone_open()
* remove spurious fput() after filp_close()
* Add documentation for the F option

Original cover letter:

I've recently been playing a lot with architecture emulation containers using qemu, mainly so I can build and test the arm secure boot toolchain on my x86 laptop (not having any arm server hardware).

The process for bringing up architecture emulation containers using qemu-user is very cumbersome because the emulation has to be installed within the container. This is bad for several reasons, firstly because it contaminates the container image to have an emulator sitting inside it. Secondly it means that all orchestration systems have to be explicitly modified to work with non-native architectures and thirdly it means that the consumer of the container can accidentally destroy the emulation and thus permanently crash the container.

The fix for this is to add a mode to binfmt_misc where the emulation can be permanently installed from the current mount namespace and where it survives both chroot and changes of mount namespace, effectively allowing the container to run using the emulator, but where the emulator itself isn't present within the container.

There are a couple of downsides to this, firstly the mapping of the interpreter is accessible inside the container even if the actual file isn't (I don't think this is really a security problem) and secondly, to update the emulator package, you now have to remove the emulation update and reinstall it, but this should be easy to do with packaging scripts. Finally, the emulator must be static otherwise the container would crash when the elf binary format attempted to run the elf interpreter.

I'm not really a regular FS developer, so I'd appreciate FS people taking a look at the new filp_clone_open() API and whether I got everything correct.

Thanks,

James

---

James Bottomley (3):
fs: add filp_clone_open API
binfmt_misc: add persistent opened binary handler for containers
binfmt_misc: add F option description to documentation

 Documentation/binfmt_misc.txt |  7 +++++++
 fs/binfmt_misc.c              | 41 +++++++++++++++++++++++++++++++++++++++--
 fs/internal.h                 |  1 +
 fs/open.c                     | 20 ++++++++++++++++++++
 4 files changed, 67 insertions(+), 2 deletions(-)

A message that is not in the archive (sent off-list or not yet received): 1456428890.2377.2.camel-d9PhHud1JfjCXq6kfMZ53/[email protected]

James Bottomley

2016-02-25 19:36 UTC #2 lore raw

PATCH v2 1/3 fs: add filp_clone_open API

I need an API that allows me to obtain a clone of the current file pointer to pass in to an exec handler. I've labelled this as an internal API because I can't see how it would be useful outside of the fs subsystem. The use case will be a persistent binfmt_misc handler.

Signed-off-by James Bottomley <James.Bottomley-d9PhHud1JfjCXq6kfMZ53/[email protected]>

---
 fs/internal.h |  1 +
 fs/open.c     | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+)

fs/internal.h +1 -0

index b71deee..c8ca0c9 100644
@@ -108,6 +108,7 @@ extern long do_handle_open(int mountdirfd,
 			   struct file_handle __user *ufh, int open_flag);
 extern int open_check_o_direct(struct file *f);
 extern int vfs_open(const struct path *, struct file *, const struct cred *);
+extern struct file *filp_clone_open(struct file *);
 
 /*
  * inode.c

fs/open.c +20 -0

index 55bdc75..bb7ffd6 100644
@@ -1004,6 +1004,26 @@ struct file *file_open_root(struct dentry *dentry, struct vfsmount *mnt,
 }
 EXPORT_SYMBOL(file_open_root);
 
+struct file *filp_clone_open(struct file *oldfile)
+{
+	struct file *file;
+	int retval;
+
+	file = get_empty_filp();
+	if (IS_ERR(file))
+		return file;
+
+	file->f_flags = oldfile->f_flags;
+	retval = vfs_open(&oldfile->f_path, file, oldfile->f_cred);
+	if (retval) {
+		fput(file);
+		return ERR_PTR(retval);
+	}
+
+	return file;
+}
+EXPORT_SYMBOL(filp_clone_open);
+
 long do_sys_open(int dfd, const char __user *filename, int flags, umode_t mode)
 {
 	struct open_flags op;

Signature · 1 line

2.6.2

James Bottomley

2016-02-25 19:37 UTC #3 lore raw

PATCH v2 2/3 binfmt_misc: add persistent opened binary handler for containers

In reply to a message not in the archive

This patch adds a new flag 'F' to the binfmt handlers. If you pass in 'F' the binary that runs the emulation will be opened immediately and in future, will be cloned from the open file.

The net effect is that the handler survives both changeroots and mount namespace changes, making it easy to work with foreign architecture containers without contaminating the container image with the emulator.

Signed-off-by James Bottomley <James.Bottomley-d9PhHud1JfjCXq6kfMZ53/[email protected]>

---
 fs/binfmt_misc.c | 41 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 39 insertions(+), 2 deletions(-)

fs/binfmt_misc.c +39 -2

index 3a3ced7..8a108c4 100644
@@ -26,6 +26,8 @@
 #include <linux/fs.h>
 #include <linux/uaccess.h>
 
+#include "internal.h"
+
 #ifdef DEBUG
 # define USE_DEBUG 1
 #else

@@ -43,6 +45,7 @@ enum {Enabled, Magic};
 #define MISC_FMT_PRESERVE_ARGV0 (1 << 31)
 #define MISC_FMT_OPEN_BINARY (1 << 30)
 #define MISC_FMT_CREDENTIALS (1 << 29)
+#define MISC_FMT_OPEN_FILE (1 << 28)
 
 typedef struct {
 	struct list_head list;

@@ -54,6 +57,7 @@ typedef struct {
 	char *interpreter;		/* filename of interpreter */
 	char *name;
 	struct dentry *dentry;
+	struct file *interp_file;
 } Node;
 
 static DEFINE_RWLOCK(entries_lock);

@@ -201,7 +205,13 @@ static int load_misc_binary(struct linux_binprm *bprm)
 	if (retval < 0)
 		goto error;
 
-	interp_file = open_exec(iname);
+	if (fmt->flags & MISC_FMT_OPEN_FILE && fmt->interp_file) {
+		interp_file = filp_clone_open(fmt->interp_file);
+		if (!IS_ERR(interp_file))
+			deny_write_access(interp_file);
+	} else {
+		interp_file = open_exec(iname);
+	}
 	retval = PTR_ERR(interp_file);
 	if (IS_ERR(interp_file))
 		goto error;

@@ -285,6 +295,11 @@ static char *check_special_flags(char *sfs, Node *e)
 			e->flags |= (MISC_FMT_CREDENTIALS |
 					MISC_FMT_OPEN_BINARY);
 			break;
+		case 'F':
+			pr_debug("register: flag: F: open interpreter file now\n");
+			p++;
+			e->flags |= MISC_FMT_OPEN_FILE;
+			break;
 		default:
 			cont = 0;
 		}

@@ -543,6 +558,8 @@ static void entry_status(Node *e, char *page)
 		*dp++ = 'O';
 	if (e->flags & MISC_FMT_CREDENTIALS)
 		*dp++ = 'C';
+	if (e->flags & MISC_FMT_OPEN_FILE)
+		*dp++ = 'F';
 	*dp++ = '\n';
 
 	if (!test_bit(Magic, &e->flags)) {

@@ -590,6 +607,11 @@ static void kill_node(Node *e)
 	}
 	write_unlock(&entries_lock);
 
+	if ((e->flags & MISC_FMT_OPEN_FILE) && e->interp_file) {
+		filp_close(e->interp_file, NULL);
+		e->interp_file = NULL;
+	}
+
 	if (dentry) {
 		drop_nlink(d_inode(dentry));
 		d_drop(dentry);

@@ -698,6 +720,21 @@ static ssize_t bm_register_write(struct file *file, const char __user *buffer,
 		goto out2;
 	}
 
+	if (e->flags & MISC_FMT_OPEN_FILE) {
+		struct file *f;
+
+		f = open_exec(e->interpreter);
+		if (IS_ERR(f)) {
+			err = PTR_ERR(f);
+			pr_notice("register: failed to install interpreter file %s\n", e->interpreter);
+			simple_release_fs(&bm_mnt, &entry_count);
+			iput(inode);
+			inode = NULL;
+			goto out2;
+		}
+		e->interp_file = f;
+	}
+
 	e->dentry = dget(dentry);
 	inode->i_private = e;
 	inode->i_fop = &bm_entry_operations;

@@ -716,7 +753,7 @@ out:
 
 	if (err) {
 		kfree(e);
-		return -EINVAL;
+		return err;
 	}
 	return count;
 }

Signature · 1 line

2.6.2

James Bottomley

author

2016-02-25 19:36 UTC #4 lore raw

PATCH v2 1/3 fs: add filp_clone_open API

In reply to James Bottomley

Signed-off-by James Bottomley <[email protected]>

---
 fs/internal.h |  1 +
 fs/open.c     | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+)

fs/internal.h +1 -0

index b71deee..c8ca0c9 100644
@@ -108,6 +108,7 @@ extern long do_handle_open(int mountdirfd,
 			   struct file_handle __user *ufh, int open_flag);
 extern int open_check_o_direct(struct file *f);
 extern int vfs_open(const struct path *, struct file *, const struct cred *);
+extern struct file *filp_clone_open(struct file *);
 
 /*
  * inode.c

fs/open.c +20 -0

index 55bdc75..bb7ffd6 100644
@@ -1004,6 +1004,26 @@ struct file *file_open_root(struct dentry *dentry, struct vfsmount *mnt,
 }
 EXPORT_SYMBOL(file_open_root);
 
+struct file *filp_clone_open(struct file *oldfile)
+{
+	struct file *file;
+	int retval;
+
+	file = get_empty_filp();
+	if (IS_ERR(file))
+		return file;
+
+	file->f_flags = oldfile->f_flags;
+	retval = vfs_open(&oldfile->f_path, file, oldfile->f_cred);
+	if (retval) {
+		fput(file);
+		return ERR_PTR(retval);
+	}
+
+	return file;
+}
+EXPORT_SYMBOL(filp_clone_open);
+
 long do_sys_open(int dfd, const char __user *filename, int flags, umode_t mode)
 {
 	struct open_flags op;

Signature · 1 line

2.6.2

Serge E. Hallyn

Acked-by Serge Hallyn on v2 1/3

2016-03-08 06:19 UTC #5 lore raw

Re: [Patch v2 1/3] fs: add filp_clone_open API

Quoted 6 lines · James Bottomley

I need an API that allows me to obtain a clone of the current file pointer to pass in to an exec handler. I've labelled this as an internal API because I can't see how it would be useful outside of the fs subsystem. The use case will be a persistent binfmt_misc handler.
Signed-off-by James Bottomley <James.Bottomley-d9PhHud1JfjCXq6kfMZ53/[email protected]>

Thanks, James, worthwhlie feature.

Acked-by Serge Hallyn <[email protected]>

Quoted 55 lines

---
 fs/internal.h |  1 +
 fs/open.c     | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+)

fs/internal.h +1 -0

index b71deee..c8ca0c9 100644
@@ -108,6 +108,7 @@ extern long do_handle_open(int mountdirfd,
 			   struct file_handle __user *ufh, int open_flag);
 extern int open_check_o_direct(struct file *f);
 extern int vfs_open(const struct path *, struct file *, const struct cred *);
+extern struct file *filp_clone_open(struct file *);
 
 /*
  * inode.c

fs/open.c +20 -0

index 55bdc75..bb7ffd6 100644
@@ -1004,6 +1004,26 @@ struct file *file_open_root(struct dentry *dentry, struct vfsmount *mnt,
 }
 EXPORT_SYMBOL(file_open_root);
 
+struct file *filp_clone_open(struct file *oldfile)
+{
+	struct file *file;
+	int retval;
+
+	file = get_empty_filp();
+	if (IS_ERR(file))
+		return file;
+
+	file->f_flags = oldfile->f_flags;
+	retval = vfs_open(&oldfile->f_path, file, oldfile->f_cred);
+	if (retval) {
+		fput(file);
+		return ERR_PTR(retval);
+	}
+
+	return file;
+}
+EXPORT_SYMBOL(filp_clone_open);
+
 long do_sys_open(int dfd, const char __user *filename, int flags, umode_t mode)
 {
 	struct open_flags op;

Signature · 6 lines

2.6.2

_______________________________________________
Containers mailing list
Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
https://lists.linuxfoundation.org/mailman/listinfo/containers

Mateusz Guzik

2016-03-08 09:17 UTC #6 lore raw

Re: [Patch v2 1/3] fs: add filp_clone_open API

In reply to James Bottomley

Quoted 34 lines · James Bottomley

I need an API that allows me to obtain a clone of the current file pointer to pass in to an exec handler. I've labelled this as an internal API because I can't see how it would be useful outside of the fs subsystem. The use case will be a persistent binfmt_misc handler.
Signed-off-by James Bottomley <James.Bottomley-d9PhHud1JfjCXq6kfMZ53/[email protected]>
---
 fs/internal.h |  1 +
 fs/open.c     | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+)
fs/internal.h +1 -0
index b71deee..c8ca0c9 100644
@@ -108,6 +108,7 @@ extern long do_handle_open(int mountdirfd,
 			   struct file_handle __user *ufh, int open_flag);
 extern int open_check_o_direct(struct file *f);
 extern int vfs_open(const struct path *, struct file *, const struct cred *);
+extern struct file *filp_clone_open(struct file *);
 
 /*
  * inode.c
fs/open.c +3 -0
index 55bdc75..bb7ffd6 100644
@@ -1004,6 +1004,26 @@ struct file *file_open_root(struct dentry *dentry, struct vfsmount *mnt,
 }
 EXPORT_SYMBOL(file_open_root);
 
+struct file *filp_clone_open(struct file *oldfile)
+{
+	struct file *file;

fs/open.c struct file *file_open_root(struct dentry *dentry, struct vfsmount *mnt, · line 1019

Quoted 10 lines

fs/open.c +10 -0

+	int retval;
+
+	file = get_empty_filp();
+	if (IS_ERR(file))
+		return file;
+
+	file->f_flags = oldfile->f_flags;
+	retval = vfs_open(&oldfile->f_path, file, oldfile->f_cred);
+	if (retval) {
+		fput(file);

You likely want put_file instead, although if I read it right this is only cosmetics for consistency with dentry_open.

Also vast majority of the file uses 'error' instead of 'retval'. Any reason to deviate here?

Quoted 13 lines

fs/open.c +7 -0

+		return ERR_PTR(retval);
+	}
+
+	return file;
+}
+EXPORT_SYMBOL(filp_clone_open);
+
 long do_sys_open(int dfd, const char __user *filename, int flags, umode_t mode)
 {
 	struct open_flags op;

Signature · 1 line

2.6.2

Signature · 1 line

Mateusz Guzik

James Bottomley

2016-03-08 11:19 UTC #7 lore raw

Re: [Patch v2 1/3] fs: add filp_clone_open API

Quoted 47 lines · Mateusz Guzik

Quoted 46 lines · James Bottomley
I need an API that allows me to obtain a clone of the current file pointer to pass in to an exec handler. I've labelled this as an internal API because I can't see how it would be useful outside of the
fs subsystem. The use case will be a persistent binfmt_misc handler.
Signed-off-by: James Bottomley < James.Bottomley-d9PhHud1JfjCXq6kfMZ53/[email protected]>
---
 fs/internal.h |  1 +
 fs/open.c     | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+)
fs/internal.h +1 -0
index b71deee..c8ca0c9 100644
@@ -108,6 +108,7 @@ extern long do_handle_open(int mountdirfd,
 			   struct file_handle __user *ufh, int
 open_flag);
 extern int open_check_o_direct(struct file *f);
 extern int vfs_open(const struct path *, struct file *, const
 struct cred *);
+extern struct file *filp_clone_open(struct file *);
 
/*
* inode.c
fs/open.c +9 -0
index 55bdc75..bb7ffd6 100644
@@ -1004,6 +1004,26 @@ struct file *file_open_root(struct dentry
 *dentry, struct vfsmount *mnt,
 }
 EXPORT_SYMBOL(file_open_root);
 
+struct file *filp_clone_open(struct file *oldfile)
+{
+	struct file *file;
+	int retval;
+
+	file = get_empty_filp();
+	if (IS_ERR(file))
+		return file;
+

Quoted 8 lines

fs/open.c struct file *file_open_root(struct dentry *dentry, struct vfsmount *mnt, · line 1019
Quoted 5 lines
fs/open.c +4 -1
+	file->f_flags = oldfile->f_flags;
+	retval = vfs_open(&oldfile->f_path, file, oldfile
->f_cred);
+	if (retval) {
+		fput(file);
You likely want put_file instead, although if I read it right this is only cosmetics for consistency with dentry_open.

get_empty_filp() is always paired with fput(). There isn't a put_file(); there's a put_files_struct() but that's for something different.

Quoted 2 lines

Also vast majority of the file uses 'error' instead of 'retval'. Any reason to deviate here?

it mirrors the close routines which also use retval.

James

Quoted 15 lines

Quoted 14 lines

fs/open.c +7 -0

+		return ERR_PTR(retval);
+	}
+
+	return file;
+}
+EXPORT_SYMBOL(filp_clone_open);
+
 long do_sys_open(int dfd, const char __user *filename, int flags,

umode_t mode)
{
struct open_flags op;

Signature · 1 line

2.6.2

James Bottomley

author

2016-03-08 11:19 UTC #8 lore raw

Re: [Patch v2 1/3] fs: add filp_clone_open API

In reply to Mateusz Guzik

Quoted 47 lines · Mateusz Guzik

Quoted 46 lines · James Bottomley

I need an API that allows me to obtain a clone of the current file pointer to pass in to an exec handler. I've labelled this as an internal API because I can't see how it would be useful outside of the
fs subsystem. The use case will be a persistent binfmt_misc handler.

Signed-off-by: James Bottomley <
[email protected]>

---
 fs/internal.h |  1 +
 fs/open.c     | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+)

fs/internal.h +1 -0

index b71deee..c8ca0c9 100644
@@ -108,6 +108,7 @@ extern long do_handle_open(int mountdirfd,
 			   struct file_handle __user *ufh, int
 open_flag);
 extern int open_check_o_direct(struct file *f);
 extern int vfs_open(const struct path *, struct file *, const
 struct cred *);
+extern struct file *filp_clone_open(struct file *);

/*
* inode.c

fs/open.c +9 -0

index 55bdc75..bb7ffd6 100644
@@ -1004,6 +1004,26 @@ struct file *file_open_root(struct dentry
 *dentry, struct vfsmount *mnt,
 }
 EXPORT_SYMBOL(file_open_root);
 
+struct file *filp_clone_open(struct file *oldfile)
+{
+	struct file *file;
+	int retval;
+
+	file = get_empty_filp();
+	if (IS_ERR(file))
+		return file;
+

Quoted 8 lines

fs/open.c struct file *file_open_root(struct dentry *dentry, struct vfsmount *mnt, · line 1019
Quoted 5 lines
fs/open.c +4 -1
+	file->f_flags = oldfile->f_flags;
+	retval = vfs_open(&oldfile->f_path, file, oldfile
->f_cred);
+	if (retval) {
+		fput(file);
You likely want put_file instead, although if I read it right this is only cosmetics for consistency with dentry_open.

get_empty_filp() is always paired with fput(). There isn't a put_file(); there's a put_files_struct() but that's for something different.

Quoted 2 lines

Also vast majority of the file uses 'error' instead of 'retval'. Any reason to deviate here?

it mirrors the close routines which also use retval.

James

Quoted 15 lines

Quoted 14 lines

fs/open.c +7 -0

+		return ERR_PTR(retval);
+	}
+
+	return file;
+}
+EXPORT_SYMBOL(filp_clone_open);
+
 long do_sys_open(int dfd, const char __user *filename, int flags,

umode_t mode)
{
struct open_flags op;

Signature · 1 line

2.6.2

James Bottomley

author

2016-02-25 19:37 UTC #9 lore raw

PATCH v2 2/3 binfmt_misc: add persistent opened binary handler for containers

In reply to James Bottomley

This patch adds a new flag 'F' to the binfmt handlers. If you pass in 'F' the binary that runs the emulation will be opened immediately and in future, will be cloned from the open file.

Signed-off-by James Bottomley <[email protected]>

---
 fs/binfmt_misc.c | 41 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 39 insertions(+), 2 deletions(-)

fs/binfmt_misc.c +39 -2

index 3a3ced7..8a108c4 100644
@@ -26,6 +26,8 @@
 #include <linux/fs.h>
 #include <linux/uaccess.h>
 
+#include "internal.h"
+
 #ifdef DEBUG
 # define USE_DEBUG 1
 #else

@@ -43,6 +45,7 @@ enum {Enabled, Magic};
 #define MISC_FMT_PRESERVE_ARGV0 (1 << 31)
 #define MISC_FMT_OPEN_BINARY (1 << 30)
 #define MISC_FMT_CREDENTIALS (1 << 29)
+#define MISC_FMT_OPEN_FILE (1 << 28)
 
 typedef struct {
 	struct list_head list;

@@ -54,6 +57,7 @@ typedef struct {
 	char *interpreter;		/* filename of interpreter */
 	char *name;
 	struct dentry *dentry;
+	struct file *interp_file;
 } Node;
 
 static DEFINE_RWLOCK(entries_lock);

@@ -201,7 +205,13 @@ static int load_misc_binary(struct linux_binprm *bprm)
 	if (retval < 0)
 		goto error;
 
-	interp_file = open_exec(iname);
+	if (fmt->flags & MISC_FMT_OPEN_FILE && fmt->interp_file) {
+		interp_file = filp_clone_open(fmt->interp_file);
+		if (!IS_ERR(interp_file))
+			deny_write_access(interp_file);
+	} else {
+		interp_file = open_exec(iname);
+	}
 	retval = PTR_ERR(interp_file);
 	if (IS_ERR(interp_file))
 		goto error;

@@ -285,6 +295,11 @@ static char *check_special_flags(char *sfs, Node *e)
 			e->flags |= (MISC_FMT_CREDENTIALS |
 					MISC_FMT_OPEN_BINARY);
 			break;
+		case 'F':
+			pr_debug("register: flag: F: open interpreter file now\n");
+			p++;
+			e->flags |= MISC_FMT_OPEN_FILE;
+			break;
 		default:
 			cont = 0;
 		}

@@ -543,6 +558,8 @@ static void entry_status(Node *e, char *page)
 		*dp++ = 'O';
 	if (e->flags & MISC_FMT_CREDENTIALS)
 		*dp++ = 'C';
+	if (e->flags & MISC_FMT_OPEN_FILE)
+		*dp++ = 'F';
 	*dp++ = '\n';
 
 	if (!test_bit(Magic, &e->flags)) {

@@ -590,6 +607,11 @@ static void kill_node(Node *e)
 	}
 	write_unlock(&entries_lock);
 
+	if ((e->flags & MISC_FMT_OPEN_FILE) && e->interp_file) {
+		filp_close(e->interp_file, NULL);
+		e->interp_file = NULL;
+	}
+
 	if (dentry) {
 		drop_nlink(d_inode(dentry));
 		d_drop(dentry);

@@ -698,6 +720,21 @@ static ssize_t bm_register_write(struct file *file, const char __user *buffer,
 		goto out2;
 	}
 
+	if (e->flags & MISC_FMT_OPEN_FILE) {
+		struct file *f;
+
+		f = open_exec(e->interpreter);
+		if (IS_ERR(f)) {
+			err = PTR_ERR(f);
+			pr_notice("register: failed to install interpreter file %s\n", e->interpreter);
+			simple_release_fs(&bm_mnt, &entry_count);
+			iput(inode);
+			inode = NULL;
+			goto out2;
+		}
+		e->interp_file = f;
+	}
+
 	e->dentry = dget(dentry);
 	inode->i_private = e;
 	inode->i_fop = &bm_entry_operations;

@@ -716,7 +753,7 @@ out:
 
 	if (err) {
 		kfree(e);
-		return -EINVAL;
+		return err;
 	}
 	return count;
 }

Signature · 1 line

2.6.2

A message that is not in the archive (sent off-list or not yet received): 1456429071.2377.5.camel-d9PhHud1JfjCXq6kfMZ53/[email protected]

Serge E. Hallyn

Acked-by Serge Hallyn

2016-03-08 08:10 UTC #11 lore raw

Re: [Patch v2 2/3] binfmt_misc: add persistent opened binary handler for containers

Quoted 10 lines · James Bottomley

This patch adds a new flag 'F' to the binfmt handlers. If you pass in 'F' the binary that runs the emulation will be opened immediately and in future, will be cloned from the open file.
The net effect is that the handler survives both changeroots and mount namespace changes, making it easy to work with foreign architecture containers without contaminating the container image with the emulator.
Signed-off-by James Bottomley <James.Bottomley-d9PhHud1JfjCXq6kfMZ53/[email protected]>

Acked-by Serge Hallyn <[email protected]>

Quoted 119 lines

---
 fs/binfmt_misc.c | 41 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 39 insertions(+), 2 deletions(-)

fs/binfmt_misc.c +39 -2

index 3a3ced7..8a108c4 100644
@@ -26,6 +26,8 @@
 #include <linux/fs.h>
 #include <linux/uaccess.h>
 
+#include "internal.h"
+
 #ifdef DEBUG
 # define USE_DEBUG 1
 #else

@@ -43,6 +45,7 @@ enum {Enabled, Magic};
 #define MISC_FMT_PRESERVE_ARGV0 (1 << 31)
 #define MISC_FMT_OPEN_BINARY (1 << 30)
 #define MISC_FMT_CREDENTIALS (1 << 29)
+#define MISC_FMT_OPEN_FILE (1 << 28)
 
 typedef struct {
 	struct list_head list;

@@ -54,6 +57,7 @@ typedef struct {
 	char *interpreter;		/* filename of interpreter */
 	char *name;
 	struct dentry *dentry;
+	struct file *interp_file;
 } Node;
 
 static DEFINE_RWLOCK(entries_lock);

@@ -201,7 +205,13 @@ static int load_misc_binary(struct linux_binprm *bprm)
 	if (retval < 0)
 		goto error;
 
-	interp_file = open_exec(iname);
+	if (fmt->flags & MISC_FMT_OPEN_FILE && fmt->interp_file) {
+		interp_file = filp_clone_open(fmt->interp_file);
+		if (!IS_ERR(interp_file))
+			deny_write_access(interp_file);
+	} else {
+		interp_file = open_exec(iname);
+	}
 	retval = PTR_ERR(interp_file);
 	if (IS_ERR(interp_file))
 		goto error;

@@ -285,6 +295,11 @@ static char *check_special_flags(char *sfs, Node *e)
 			e->flags |= (MISC_FMT_CREDENTIALS |
 					MISC_FMT_OPEN_BINARY);
 			break;
+		case 'F':
+			pr_debug("register: flag: F: open interpreter file now\n");
+			p++;
+			e->flags |= MISC_FMT_OPEN_FILE;
+			break;
 		default:
 			cont = 0;
 		}

@@ -543,6 +558,8 @@ static void entry_status(Node *e, char *page)
 		*dp++ = 'O';
 	if (e->flags & MISC_FMT_CREDENTIALS)
 		*dp++ = 'C';
+	if (e->flags & MISC_FMT_OPEN_FILE)
+		*dp++ = 'F';
 	*dp++ = '\n';
 
 	if (!test_bit(Magic, &e->flags)) {

@@ -590,6 +607,11 @@ static void kill_node(Node *e)
 	}
 	write_unlock(&entries_lock);
 
+	if ((e->flags & MISC_FMT_OPEN_FILE) && e->interp_file) {
+		filp_close(e->interp_file, NULL);
+		e->interp_file = NULL;
+	}
+
 	if (dentry) {
 		drop_nlink(d_inode(dentry));
 		d_drop(dentry);

@@ -698,6 +720,21 @@ static ssize_t bm_register_write(struct file *file, const char __user *buffer,
 		goto out2;
 	}
 
+	if (e->flags & MISC_FMT_OPEN_FILE) {
+		struct file *f;
+
+		f = open_exec(e->interpreter);
+		if (IS_ERR(f)) {
+			err = PTR_ERR(f);
+			pr_notice("register: failed to install interpreter file %s\n", e->interpreter);
+			simple_release_fs(&bm_mnt, &entry_count);
+			iput(inode);
+			inode = NULL;
+			goto out2;
+		}
+		e->interp_file = f;
+	}
+
 	e->dentry = dget(dentry);
 	inode->i_private = e;
 	inode->i_fop = &bm_entry_operations;

@@ -716,7 +753,7 @@ out:
 
 	if (err) {
 		kfree(e);
-		return -EINVAL;
+		return err;
 	}
 	return count;
 }

Signature · 6 lines

2.6.2

_______________________________________________
Containers mailing list
Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
https://lists.linuxfoundation.org/mailman/listinfo/containers

Serge E. Hallyn

Acked-by Serge Hallyn on v2 2/3

2016-03-08 08:10 UTC #12 lore raw

Re: [Patch v2 2/3] binfmt_misc: add persistent opened binary handler for containers

In reply to James Bottomley

Quoted 10 lines · James Bottomley

This patch adds a new flag 'F' to the binfmt handlers. If you pass in 'F' the binary that runs the emulation will be opened immediately and in future, will be cloned from the open file.
The net effect is that the handler survives both changeroots and mount namespace changes, making it easy to work with foreign architecture containers without contaminating the container image with the emulator.
Signed-off-by James Bottomley <[email protected]>

Acked-by Serge Hallyn <[email protected]>

Quoted 119 lines

---
 fs/binfmt_misc.c | 41 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 39 insertions(+), 2 deletions(-)

fs/binfmt_misc.c +39 -2

index 3a3ced7..8a108c4 100644
@@ -26,6 +26,8 @@
 #include <linux/fs.h>
 #include <linux/uaccess.h>
 
+#include "internal.h"
+
 #ifdef DEBUG
 # define USE_DEBUG 1
 #else

@@ -43,6 +45,7 @@ enum {Enabled, Magic};
 #define MISC_FMT_PRESERVE_ARGV0 (1 << 31)
 #define MISC_FMT_OPEN_BINARY (1 << 30)
 #define MISC_FMT_CREDENTIALS (1 << 29)
+#define MISC_FMT_OPEN_FILE (1 << 28)
 
 typedef struct {
 	struct list_head list;

@@ -54,6 +57,7 @@ typedef struct {
 	char *interpreter;		/* filename of interpreter */
 	char *name;
 	struct dentry *dentry;
+	struct file *interp_file;
 } Node;
 
 static DEFINE_RWLOCK(entries_lock);

@@ -201,7 +205,13 @@ static int load_misc_binary(struct linux_binprm *bprm)
 	if (retval < 0)
 		goto error;
 
-	interp_file = open_exec(iname);
+	if (fmt->flags & MISC_FMT_OPEN_FILE && fmt->interp_file) {
+		interp_file = filp_clone_open(fmt->interp_file);
+		if (!IS_ERR(interp_file))
+			deny_write_access(interp_file);
+	} else {
+		interp_file = open_exec(iname);
+	}
 	retval = PTR_ERR(interp_file);
 	if (IS_ERR(interp_file))
 		goto error;

@@ -285,6 +295,11 @@ static char *check_special_flags(char *sfs, Node *e)
 			e->flags |= (MISC_FMT_CREDENTIALS |
 					MISC_FMT_OPEN_BINARY);
 			break;
+		case 'F':
+			pr_debug("register: flag: F: open interpreter file now\n");
+			p++;
+			e->flags |= MISC_FMT_OPEN_FILE;
+			break;
 		default:
 			cont = 0;
 		}

@@ -543,6 +558,8 @@ static void entry_status(Node *e, char *page)
 		*dp++ = 'O';
 	if (e->flags & MISC_FMT_CREDENTIALS)
 		*dp++ = 'C';
+	if (e->flags & MISC_FMT_OPEN_FILE)
+		*dp++ = 'F';
 	*dp++ = '\n';
 
 	if (!test_bit(Magic, &e->flags)) {

@@ -590,6 +607,11 @@ static void kill_node(Node *e)
 	}
 	write_unlock(&entries_lock);
 
+	if ((e->flags & MISC_FMT_OPEN_FILE) && e->interp_file) {
+		filp_close(e->interp_file, NULL);
+		e->interp_file = NULL;
+	}
+
 	if (dentry) {
 		drop_nlink(d_inode(dentry));
 		d_drop(dentry);

@@ -698,6 +720,21 @@ static ssize_t bm_register_write(struct file *file, const char __user *buffer,
 		goto out2;
 	}
 
+	if (e->flags & MISC_FMT_OPEN_FILE) {
+		struct file *f;
+
+		f = open_exec(e->interpreter);
+		if (IS_ERR(f)) {
+			err = PTR_ERR(f);
+			pr_notice("register: failed to install interpreter file %s\n", e->interpreter);
+			simple_release_fs(&bm_mnt, &entry_count);
+			iput(inode);
+			inode = NULL;
+			goto out2;
+		}
+		e->interp_file = f;
+	}
+
 	e->dentry = dget(dentry);
 	inode->i_private = e;
 	inode->i_fop = &bm_entry_operations;

@@ -716,7 +753,7 @@ out:
 
 	if (err) {
 		kfree(e);
-		return -EINVAL;
+		return err;
 	}
 	return count;
 }

Signature · 6 lines

2.6.2

_______________________________________________
Containers mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/containers

James Bottomley

author

2016-02-25 19:38 UTC #13 lore raw

PATCH v2 3/3 binfmt_misc: add F option description to documentation

In reply to James Bottomley

Signed-off-by James Bottomley <James.Bottomley-d9PhHud1JfjCXq6kfMZ53/[email protected]>

---
 Documentation/binfmt_misc.txt | 7 +++++++
 1 file changed, 7 insertions(+)

Documentation/binfmt_misc.txt +7 -0

index 6b1de70..a22eb69 100644
@@ -66,6 +66,13 @@ Here is what the fields mean:
             This feature should be used with care as the interpreter
             will run with root permissions when a setuid binary owned by root
             is run with binfmt_misc.
+      'F' - fix binary.  The usual behaviour of binfmt_misc is to spawn the
+      	    binary lazily when the misc format file is invoked.  However,
+	    this doesn't work very well in the face of mount namespaces and
+	    changeroots, so the F mode opens the binary as soon as the
+	    emultation is installed and uses the opened image to spawn the
+	    emulator, meaning it is always available once installed,
+	    regardless of how the environment changes.
 
 
 There are some restrictions:

Signature · 1 line

2.6.2

A message that is not in the archive (sent off-list or not yet received): 1456429136.2377.7.camel-d9PhHud1JfjCXq6kfMZ53/[email protected]

Randy Dunlap

2016-02-29 05:32 UTC #15 lore raw

Re: [Patch v2 3/3] binfmt_misc: add F option description to documentation

Quoted 10 lines · James Bottomley

Signed-off-by James Bottomley <James.Bottomley-d9PhHud1JfjCXq6kfMZ53/[email protected]>
---
 Documentation/binfmt_misc.txt | 7 +++++++
 1 file changed, 7 insertions(+)
Documentation/binfmt_misc.txt +0 -0
index 6b1de70..a22eb69 100644

Documentation/binfmt_misc.txt Here is what the fields mean: · line 73

Quoted 8 lines

Documentation/binfmt_misc.txt +5 -0

@@ -66,6 +66,13 @@ Here is what the fields mean:
             This feature should be used with care as the interpreter
             will run with root permissions when a setuid binary owned by root
             is run with binfmt_misc.
+      'F' - fix binary.  The usual behaviour of binfmt_misc is to spawn the
+      	    binary lazily when the misc format file is invoked.  However,
+	    this doesn't work very well in the face of mount namespaces and
+	    changeroots, so the F mode opens the binary as soon as the
+	    emultation is installed and uses the opened image to spawn the

emulation

Quoted 6 lines

Documentation/binfmt_misc.txt +2 -0

+	    emulator, meaning it is always available once installed,
+	    regardless of how the environment changes.
 
 
 There are some restrictions:

Signature · 1 line

~Randy

Randy Dunlap

reviewer

2016-02-29 05:32 UTC #16 lore raw

Re: [Patch v2 3/3] binfmt_misc: add F option description to documentation

In reply to James Bottomley

Quoted 10 lines · James Bottomley

Signed-off-by James Bottomley <[email protected]>
---
 Documentation/binfmt_misc.txt | 7 +++++++
 1 file changed, 7 insertions(+)
Documentation/binfmt_misc.txt +0 -0
index 6b1de70..a22eb69 100644

Documentation/binfmt_misc.txt Here is what the fields mean: · line 73

Quoted 8 lines

Documentation/binfmt_misc.txt +5 -0

@@ -66,6 +66,13 @@ Here is what the fields mean:
             This feature should be used with care as the interpreter
             will run with root permissions when a setuid binary owned by root
             is run with binfmt_misc.
+      'F' - fix binary.  The usual behaviour of binfmt_misc is to spawn the
+      	    binary lazily when the misc format file is invoked.  However,
+	    this doesn't work very well in the face of mount namespaces and
+	    changeroots, so the F mode opens the binary as soon as the
+	    emultation is installed and uses the opened image to spawn the

emulation

Quoted 6 lines

Documentation/binfmt_misc.txt +2 -0

+	    emulator, meaning it is always available once installed,
+	    regardless of how the environment changes.
 
 
 There are some restrictions:

Signature · 1 line

~Randy