On Thu, Jan 10, 2019 at 01:19:27PM +0100, Stefan Agner wrote:
> Using update-binfmts with the same magic and fix-binary twice, will
> install the detector on the second call. However, this breaks fix-binary
> since the whole point of fixed binary is to allow to use the interpreter
> inside a container/chroot. If a detector is in the play, this detector
> won't be able to load the interpreter from inside the container/chroot.
>
> I think it would be more sensible for binfmt-support to warn the user on
> the second invocation.
This is a good point, thanks. I think your patch adds the warning in
slightly the wrong place, though, as it misses the case where a detector
is requested explicitly (e.g. using --detector), which has essentially
the same problem. We should probably also document the situation.
What do you think of the attached patch against upstream master?
Thanks,
--
Colin Watson [cjwat...@debian.org]
>From f55485ae190049f0cf4922133f3706ebdf4f7c5e Mon Sep 17 00:00:00 2001
From: Colin Watson <cjwat...@debian.org>
Date: Sun, 13 Jan 2019 23:26:20 +0000
Subject: [PATCH] Warn about fix-binary/detectors incompatibility
"--fix-binary yes" is incompatible with detectors. Warn the user if
they try to use both at once.
Thanks to Stefan Agner; fixes Debian bug #918901.
* src/update-binfmts.c (act_enable): Warn and return if a detector is
needed and the fix-binary flag is set.
* man/update-binfmts.man8 (BINARY FORMAT SPECIFICATIONS): Document the
incompatibility.
* NEWS: Document this.
---
NEWS | 3 +++
man/update-binfmts.man8 | 8 ++++++++
src/update-binfmts.c | 6 ++++++
3 files changed, 17 insertions(+)
diff --git a/NEWS b/NEWS
index 9414a7a..c42367e 100644
--- a/NEWS
+++ b/NEWS
@@ -15,6 +15,9 @@ Don't enable formats on import or disable them on unimport unless
/proc/sys/fs/binfmt_misc is already mounted. This avoids causing cleanup
problems in chroots.
+"--fix-binary yes" is incompatible with detectors. Warn the user if they
+try to use both at once. Thanks to Stefan Agner.
+
binfmt-support 2.1.8 (22 August 2017)
=====================================
diff --git a/man/update-binfmts.man8 b/man/update-binfmts.man8
index b0a11a8..9f5b133 100644
--- a/man/update-binfmts.man8
+++ b/man/update-binfmts.man8
@@ -256,6 +256,8 @@ This may be used when the binary format is more complex than can be handled
by the kernel's format specifications alone.
The program should return an exit code of zero if the file is appropriate
and non-zero otherwise.
+This option cannot be used together with
+.Fl Fl fix\-binary Cm yes .
.It Fl Fl credentials Cm yes , Fl Fl credentials Cm no
Whether to keep the credentials of the original binary to run the interpreter;
this is typically useful to run setuid binaries, but has security implications.
@@ -274,6 +276,12 @@ The default behaviour is
meaning that the kernel should open the interpreter binary lazily when
needed.
This option requires Linux 4.8 or newer.
+It cannot be used together with
+.Fl Fl detector ,
+or with multiple binary formats that share the same magic number, since the
+kernel will only open a single interpreter binary which will then not be
+able to detect and execute the real interpreter from inside a chroot or from
+a different mount namespace.
.El
.Ss FORMAT FILES
A format file is a sequence of options, one per line, corresponding roughly
diff --git a/src/update-binfmts.c b/src/update-binfmts.c
index f0d6526..e562b2a 100644
--- a/src/update-binfmts.c
+++ b/src/update-binfmts.c
@@ -359,6 +359,12 @@ static int act_enable (const char *name)
fix_binary =
(binfmt->fix_binary && !strcmp (binfmt->fix_binary, "yes"))
? "F" : "";
+ if (need_detector && *fix_binary) {
+ warning_err ("unable to enable binary format %s: another format "
+ "with the same magic already exists and this is a "
+ "fix-binary format", name);
+ return 0;
+ }
regstring = xasprintf (":%s:%c:%s:%s:%s:%s:%s%s%s\n",
name, type, binfmt->offset, binfmt->magic,
binfmt->mask, interpreter,
--
2.20.1
