- Package:
- binfmt-support
- Source:
- binfmt-support
- Description:
- Support for extra binary formats
- Submitter:
- Arthur Marsh
- Date:
- 2014-12-31 06:27:04 UTC
- Severity:
- important
Dear Maintainer,
*** Reporter, please consider answering these questions, where appropriate ***
* What led up to the situation?
upgrading kernel to current Linus' current git (11 December 2014)
Saw error messages:
Thu Dec 11 20:40:29 2014: [....] Enabling additional executable binary formats: binfmt-supportupdate-binfmts: warning: unable to close /proc/sys/fs/binfmt_misc/register: Invalid argument
Thu Dec 11 20:40:30 2014: update-binfmts: warning: unable to close /proc/sys/fs/binfmt_misc/register: Invalid argument
Thu Dec 11 20:40:30 2014: update-binfmts: warning: unable to close /proc/sys/fs/binfmt_misc/register: Invalid argument
Thu Dec 11 20:40:30 2014: update-binfmts: warning: unable to close /proc/sys/fs/binfmt_misc/register: Invalid argument
Thu Dec 11 20:40:30 2014: update-binfmts: warning: unable to close /proc/sys/fs/binfmt_misc/register: Invalid argument
Thu Dec 11 20:40:30 2014: update-binfmts: warning: unable to close /proc/sys/fs/binfmt_misc/register: Invalid argument
Thu Dec 11 20:40:30 2014: update-binfmts: warning: unable to close /proc/sys/fs/binfmt_misc/register: Invalid argument
Thu Dec 11 20:40:30 2014: update-binfmts: exiting due to previous errors
After this, I tried checking what was enabled and disabled:
update-binfmts --display|grep :|sort
jar (disabled):
jarwrapper (disabled):
python2.3 (disabled):
python2.6 (disabled):
python2.7 (disabled):
python3.4 (disabled):
qemu-aarch64 (enabled):
qemu-alpha (enabled):
qemu-armeb (enabled):
qemu-arm (enabled):
qemu-cris (enabled):
qemu-m68k (enabled):
qemu-microblaze (enabled):
qemu-mipsel (enabled):
qemu-mips (enabled):
qemu-ppc64abi32 (enabled):
qemu-ppc64 (enabled):
qemu-ppc64le (enabled):
qemu-ppc (enabled):
qemu-s390x (enabled):
qemu-sh4eb (enabled):
qemu-sh4 (enabled):
qemu-sparc32plus (enabled):
qemu-sparc64 (enabled):
qemu-sparc (enabled):
sun-java6 (disabled):
* What exactly did you do (or not do) that was effective (or
ineffective)?
* What was the outcome of this action?
* What outcome did you expect instead?
*** End of the template - remove these template lines ***
The latest binfmt_misc module in git has much more detailed debugging output in dmesg. What does "dmesg | grep binfmt_misc" say? Thanks,
Colin Watson wrote on 11/12/14 21:14: $ dmesg|grep binfmt_misc $ lsmod|grep binfmt_misc binfmt_misc 17691 1 $ su Password: # modinfo binfmt_misc filename: /lib/modules/3.18.0+/kernel/fs/binfmt_misc.ko license: GPL alias: fs-binfmt_misc depends: intree: Y vermagic: 3.18.0+ SMP preempt mod_unload modversions K8 Regards, Arthur.
Hm. Does your tree include https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/fs/binfmt_misc.c?id=6b899c4e9a049dfca759d990bd53b14f81c3626c ? If not, it would help to try again with that. (Hm, I guess you might need CONFIG_DYNAMIC_DEBUG. Not sure.) Thanks,
Colin Watson wrote on 11/12/14 23:10: I don't have time to git-bisect the kernel now, but reverting to 3.18.0 removed the problem. CONFIG_DYNAMIC_DEBUG was already enabled, and that commit was already included. So it might be worth running a git-bisect just on fs/binfmt_misc.c to see if one of the recent commits changed behaviour unexpectedly. Regards, Arthur.
Colin Watson wrote on 11/12/14 23:10: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=772807 Short version, on recent kernels I was seeing: Thu Dec 11 20:40:29 2014: [....] Enabling additional executable binary formats: binfmt-supportupdate-binfmts: warning: unable to close /proc/sys/fs/binfmt_misc/register: Invalid argument Thu Dec 11 20:40:30 2014: update-binfmts: warning: unable to close /proc/sys/fs/binfmt_misc/register: Invalid argument Thu Dec 11 20:40:30 2014: update-binfmts: warning: unable to close /proc/sys/fs/binfmt_misc/register: Invalid argument Thu Dec 11 20:40:30 2014: update-binfmts: warning: unable to close /proc/sys/fs/binfmt_misc/register: Invalid argument Thu Dec 11 20:40:30 2014: update-binfmts: warning: unable to close /proc/sys/fs/binfmt_misc/register: Invalid argument Thu Dec 11 20:40:30 2014: update-binfmts: warning: unable to close /proc/sys/fs/binfmt_misc/register: Invalid argument Thu Dec 11 20:40:30 2014: update-binfmts: warning: unable to close /proc/sys/fs/binfmt_misc/register: Invalid argument and only the first of several binfmt's registered (all the qemu binfmt's) when update-binfmts was run at boot time. A git-bisect revealed: git bisect good 6b899c4e9a049dfca759d990bd53b14f81c3626c is the first bad commit commit 6b899c4e9a049dfca759d990bd53b14f81c3626c Author: Mike Frysinger <vapier@gentoo.org> Date: Wed Dec 10 15:52:08 2014 -0800 binfmt_misc: add comments & debug logs When trying to develop a custom format handler, the errors returned all effectively get bucketed as EINVAL with no kernel messages. The other errors (ENOMEM/EFAULT) are internal/obvious and basic. Thus any time a bad handler is rejected, the developer has to walk the dense code and try to guess where it went wrong. Needing to dive into kernel code is itself a fairly high barrier for a lot of people. To improve this situation, let's deploy extensive pr_debug markers at logical parse points, and add comments to the dense parsing logic. It let's you see exactly where the parsing aborts, the string the kernel received (useful when dealing with shell code), how it translated the buffers to binary data, and how it will apply the mask at runtime. Some example output: $ echo ':qemu-foo:M::\x7fELF\xAD\xAD\x01\x00:\xff\xff\xff\xff\xff\x00\xff\x00:/usr/bin/qemu-foo:POC' > register $ dmesg binfmt_misc: register: received 92 bytes binfmt_misc: register: delim: 0x3a {:} binfmt_misc: register: name: {qemu-foo} binfmt_misc: register: type: M (magic) binfmt_misc: register: offset: 0x0 binfmt_misc: register: magic[raw]: 5c 78 37 66 45 4c 46 5c 78 41 44 5c 78 41 44 5c \x7fELF\xAD\xAD\ binfmt_misc: register: magic[raw]: 78 30 31 5c 78 30 30 00 x01\x00. binfmt_misc: register: mask[raw]: 5c 78 66 66 5c 78 66 66 5c 78 66 66 5c 78 66 66 \xff\xff\xff\xff binfmt_misc: register: mask[raw]: 5c 78 66 66 5c 78 30 30 5c 78 66 66 5c 78 30 30 \xff\x00\xff\x00 binfmt_misc: register: mask[raw]: 00 . binfmt_misc: register: magic/mask length: 8 binfmt_misc: register: magic[decoded]: 7f 45 4c 46 ad ad 01 00 .ELF.... binfmt_misc: register: mask[decoded]: ff ff ff ff ff 00 ff 00 ........ binfmt_misc: register: magic[masked]: 7f 45 4c 46 ad 00 01 00 .ELF.... binfmt_misc: register: interpreter: {/usr/bin/qemu-foo} binfmt_misc: register: flag: P (preserve argv0) binfmt_misc: register: flag: O (open binary) binfmt_misc: register: flag: C (preserve creds) The [raw] lines show us exactly what was received from userspace. The lines after that show us how the kernel has decoded things. Signed-off-by: Mike Frysinger <vapier@gentoo.org> Cc: Al Viro <viro@zeniv.linux.org.uk> Cc: Joe Perches <joe@perches.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> :040000 040000 d8354a4a420ed15399a6c41aa0914a8a4c6dba9a 2d491c10c9418cd16f367916f25d6050eb60152d M fs Regards, Arthur.
p[-1] = '\0';
- if (!e->magic[0])
+ if (p == e->magic)
That code makes no sense whatsoever - if p *was* equal to e->magic, the
assignment before it would be obviously broken. And a few lines later we
have another chunk just like that.
Moreover, if you look at further context, you'll see that it's
e->magic = p;
p = scanarg(p, del);
if (!p)
sod off
p[-1] = '\0';
if (p == e->magic)
sod off
Now, that condition could be true only if scanarg(p, del) would return p,
right? Let's take a look at scanarg():
static char *scanarg(char *s, char del)
{
char c;
while ((c = *s++) != del) {
if (c == '\\' && *s == 'x') {
s++;
if (!isxdigit(*s++))
return NULL;
if (!isxdigit(*s++))
return NULL;
}
}
return s;
}
See that s++ in the loop condition? There's no way in hell it would *not*
increment s. If we return non-NULL, we know that c was equal to del *and*
c is equal to previous_value_of_s[0], i.e. s[-1]. IOW, return value points
to the byte following the first instance of delimeter starting at the argument.
And p[-1] = '\0' replaces delimiter with NUL. IOW, the checks used to be
correct. And got buggered.
Subject: unfuck fs/binfmt_misc.c
Undo some of the "prettifying" braindamage.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index 70789e1..a6b6697 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -5,6 +5,8 @@
*
* binfmt_misc detects binaries via a magic or filename extension and invokes
* a specified wrapper. See Documentation/binfmt_misc.txt for more details.
+ *
+ * Cetero censeo, checkpatch.pl esse delendam
*/
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
@@ -131,9 +133,8 @@ static int load_misc_binary(struct linux_binprm *bprm)
int retval;
int fd_binary = -1;
- retval = -ENOEXEC;
if (!enabled)
- goto ret;
+ return -ENOEXEC;
/* to keep locking time low, we copy the interpreter string */
read_lock(&entries_lock);
@@ -142,12 +143,12 @@ static int load_misc_binary(struct linux_binprm *bprm)
strlcpy(iname, fmt->interpreter, BINPRM_BUF_SIZE);
read_unlock(&entries_lock);
if (!fmt)
- goto ret;
+ return -ENOEXEC;
if (!(fmt->flags & MISC_FMT_PRESERVE_ARGV0)) {
retval = remove_arg_zero(bprm);
if (retval)
- goto ret;
+ return retval;
}
if (fmt->flags & MISC_FMT_OPEN_BINARY) {
@@ -157,10 +158,9 @@ static int load_misc_binary(struct linux_binprm *bprm)
* to it
*/
fd_binary = get_unused_fd_flags(0);
- if (fd_binary < 0) {
- retval = fd_binary;
- goto ret;
- }
+ if (fd_binary < 0)
+ return fd_binary;
+
fd_install(fd_binary, bprm->file);
/* if the binary is not readable than enforce mm->dumpable=0
@@ -219,14 +219,13 @@ static int load_misc_binary(struct linux_binprm *bprm)
if (retval < 0)
goto error;
Al Viro wrote on 12/12/14 16:31: Thanks, I've successfully applied and built the patch and update-binfmts works again. Arthur.
Al Viro wrote on 12/12/14 16:31: I had to revert Al Viro's "Undo some of the "prettifying" braindamage." patch above to apply the execveat() commit below that is now in Linus' git head: https://github.com/torvalds/linux/commit/51f39a1f0cea1cacf8c787f652f26dfee9611874 After that, trying to apply Al Viro's patch resulted in on failed hunk and code that wouldn't build. binfmt_misc.c building but not working doesn't break things for me, but it would be good to get fixed. Regards, Arthur.
the checks are not correct. magic & mask are binary fields hence checking for a NUL byte to indicate string parsing failed makes no sense. your patch restores the bug i attempted to fix -- if i wanted to ignore the first byte of the file by setting the mask or magic to 0, then binfmt_misc will wrongly reject it. the current code does reject some bad inputs -- namely, when the magic or mask is not specified. i was counting on the scanarg not incrementing the pointer in that case, but as you pointed out, that doesn't work since the func always increments the pointer. i'll see if i can handle both cases. commit 7d65cf10e3d7747033b83fa18c5f3d2a498f66bc has merged at this point, but the attribution to e6084d4 is wrong. of course coding style changes & functional changes shouldn't be done which is why i didn't do it. the change you're referring to above has nothing to do e6084d4 as it was added before that in 6b899c4e9a049dfca759d990bd53b14f81c3626c (where i added extended debug output). arguably those few non-debug related lines shouldn't be in that commit, but it's a long cry from style changes. -mike