* binfmt_misc broken
@ 2013-06-10 13:52 Jeff Chua
2013-06-10 20:04 ` Linus Torvalds
2013-06-11 1:51 ` Al Viro
0 siblings, 2 replies; 4+ messages in thread
From: Jeff Chua @ 2013-06-10 13:52 UTC (permalink / raw)
To: Linux Kernel; +Cc: Linus Torvalds, Greg Kroah-Hartman, Al Viro, Andy Shevchenko
According to Documentation/binfmt_misc.txt, the 'magic' and 'mask' can be
set by echoing it to /proc/sys/fs/binfmt_misc/register.
Here's the problem I can across while working on ARM.
# echo
':arm:M::\x7fELF\x01\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x28\x00:\xff\xff\xff\xff\xff\xff\xff\x00\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff\xff:/usr/bin/qemu-arm-static:'
>/proc/sys/fs/binfmt_misc/register
# cat /proc/sys/fs/binfmt_misc/arm
wrong ...
magic 7f454c46010101
mask ffffffffffffff
right ...
magic 7f454c4601010100000000000000000002002800
mask ffffffffffffff00fffffffffffffffffeffffff
binfmt_misc is truncating e->size, so once ARM's magic is loaded, 32-bit
x86 can no longer run.
Here's a patch for it. It's looking for the delimiter ":" instead of \0.
Now 32-bit x86 can run concurrent while qemu-arm is handling ARM's magic.
Thanks,
Jeff
--- linux/fs/binfmt_misc.c 2013-05-01 12:59:39.000000000 +0800
+++ linux/fs/binfmt_misc.c 2013-06-10 21:29:45.000000000 +0800
@@ -276,6 +276,7 @@
int memsize, err;
char *buf, *p;
char del;
+ int esize = 0;
/* some sanity checks */
err = -EINVAL;
@@ -323,6 +324,15 @@
e->offset = simple_strtoul(p, &p, 10);
if (*p++)
goto Einval;
+
+ while(*(p + esize) != ':' && esize < BINPRM_BUF_SIZE) {
+ if(*(p + esize) == '\\' && *(p + esize + 1) == 'x') {
+ esize +=3;
+ } else
+ esize++;
+ }
+ e->size = esize;
+
e->magic = p;
p = scanarg(p, del);
if (!p)
@@ -337,10 +347,6 @@
p[-1] = '\0';
if (!e->mask[0])
e->mask = NULL;
- e->size = string_unescape_inplace(e->magic, UNESCAPE_HEX);
- if (e->mask &&
- string_unescape_inplace(e->mask, UNESCAPE_HEX) != e->size)
- goto Einval;
if (e->size + e->offset > BINPRM_BUF_SIZE)
goto Einval;
} else {
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: binfmt_misc broken
2013-06-10 13:52 binfmt_misc broken Jeff Chua
@ 2013-06-10 20:04 ` Linus Torvalds
2013-06-11 1:51 ` Al Viro
1 sibling, 0 replies; 4+ messages in thread
From: Linus Torvalds @ 2013-06-10 20:04 UTC (permalink / raw)
To: Jeff Chua; +Cc: Linux Kernel, Greg Kroah-Hartman, Al Viro, Andy Shevchenko
On Mon, Jun 10, 2013 at 6:52 AM, Jeff Chua <jeff.chua.linux@gmail.com> wrote:
>
> binfmt_misc is truncating e->size, so once ARM's magic is loaded, 32-bit x86
> can no longer run.
That patch is really ugly. And it doesn't make much sense. Where does
it now turn the hex into binary? And where does it check that the mask
is the same size as the data? You have changed the meaning of "esize"
to be the size of the original string, which is just wrong, and makes
no sense, since it has to be the same value for magic and for mask. So
the patch seems to make things just worse.
That said, there does seem to be *real* bugs, like "check_file()", that does
if (p && !strcmp(e->magic, p + 1))
which seems wrong. I think it should use "memcmp( ..., e->size)" instead.
And from your /proc output, esize does get truncated. But where
exactly does that happen? Is string_unescape() just broken? The code
*shouldn't* look at zeroes in the magic/mask strings, because they
should all be treated as memory regions with size 'e->size'.
Linus
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: binfmt_misc broken
2013-06-10 13:52 binfmt_misc broken Jeff Chua
2013-06-10 20:04 ` Linus Torvalds
@ 2013-06-11 1:51 ` Al Viro
2013-06-11 2:17 ` Jeff Chua
1 sibling, 1 reply; 4+ messages in thread
From: Al Viro @ 2013-06-11 1:51 UTC (permalink / raw)
To: Jeff Chua
Cc: Linux Kernel, Linus Torvalds, Greg Kroah-Hartman, Andy Shevchenko
On Mon, Jun 10, 2013 at 09:52:44PM +0800, Jeff Chua wrote:
>
>
> According to Documentation/binfmt_misc.txt, the 'magic' and 'mask'
> can be set by echoing it to /proc/sys/fs/binfmt_misc/register.
>
> Here's the problem I can across while working on ARM.
>
> # echo ':arm:M::\x7fELF\x01\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x28\x00:\xff\xff\xff\xff\xff\xff\xff\x00\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff\xff:/usr/bin/qemu-arm-static:'
> >/proc/sys/fs/binfmt_misc/register
>
> # cat /proc/sys/fs/binfmt_misc/arm
> wrong ...
> magic 7f454c46010101
> mask ffffffffffffff
>
> right ...
> magic 7f454c4601010100000000000000000002002800
> mask ffffffffffffff00fffffffffffffffffeffffff
>
>
> binfmt_misc is truncating e->size, so once ARM's magic is loaded,
> 32-bit x86 can no longer run.
>
> Here's a patch for it. It's looking for the delimiter ":" instead of
> \0. Now 32-bit x86 can run concurrent while qemu-arm is handling
> ARM's magic.
Patch is complete BS and I really wonder what kernel have you observed that bug on -
with mainline on amd64 your example yields
root@kvm-amd64:~# cat /proc/sys/fs/binfmt_misc/arm
enabled
interpreter /usr/bin/qemu-arm-static
flags:
offset 0
magic 7f454c4601010100000000000000000002002800
mask ffffffffffffff00fffffffffffffffffeffffff
A reproducer, please... As for the memcmp() Linus has suggested - it's !Magic case, i.e.
what we are comparing there is not the file contents, it's the extension. IOW, strcmp()
is the right thing to use there - pathnames do not contain NULs in the middle...
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: binfmt_misc broken
2013-06-11 1:51 ` Al Viro
@ 2013-06-11 2:17 ` Jeff Chua
0 siblings, 0 replies; 4+ messages in thread
From: Jeff Chua @ 2013-06-11 2:17 UTC (permalink / raw)
To: Al Viro; +Cc: Linux Kernel, Linus Torvalds, Greg Kroah-Hartman, Andy Shevchenko
On Tue, Jun 11, 2013 at 9:51 AM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> Patch is complete BS and I really wonder what kernel have you observed that bug on -
> with mainline on amd64 your example yields
> root@kvm-amd64:~# cat /proc/sys/fs/binfmt_misc/arm
> enabled
> interpreter /usr/bin/qemu-arm-static
> flags:
> offset 0
> magic 7f454c4601010100000000000000000002002800
> mask ffffffffffffff00fffffffffffffffffeffffff
>
> A reproducer, please... As for the memcmp() Linus has suggested - it's !Magic case, i.e.
> what we are comparing there is not the file contents, it's the extension. IOW, strcmp()
> is the right thing to use there - pathnames do not contain NULs in the middle...
BS ... yes, after testing it again, you may be right. Not intented, sorry.
I did another test with bash.
# bash -version
GNU bash, version 4.2.45(2)-release (x86_64-unknown-linux-gnu)
# echo ':arm:M::\x7fELF\x01\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x28\x00:\xff\xff\xff\xff\xff\xff\xff\x00\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff\xff:/usr/bin/qemu-arm-static:'
:arm:M::ELF(:ÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿþÿÿÿ:/usr/bin/qemu-arm-static:
# echo ':arm:M::\\x7fELF\\x01\\x01\\x01\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x02\\x00\\x28\\x00:\\xff\\xff\\xff\\xff\\xff\\xff\\xff\\x00\\xff\\xff\\xff\\xff\\xff\\xff\\xff\\xff\\xfe\\xff\\xff\\xff:/usr/bin/qemu-arm-static:'
:arm:M::\x7fELF\x01\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x28\x00:\xff\xff\xff\xff\xff\xff\xff\x00\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff\xff:/usr/bin/qemu-arm-static:
I supposed it's my bash configured with opt_xpg_echo=yes that's
sending in different data to the kernel.
Sending in the double-escape solved the problem. BS totally! My fault.
Thanks,
Jeff
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-06-11 2:17 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-10 13:52 binfmt_misc broken Jeff Chua
2013-06-10 20:04 ` Linus Torvalds
2013-06-11 1:51 ` Al Viro
2013-06-11 2:17 ` Jeff Chua
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).