qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] qemu-img segfaults on MIPS hosts due to not having an executable stack
@ 2016-06-13 14:11 Peter Maydell
  2016-06-13 14:45 ` Daniel P. Berrange
  0 siblings, 1 reply; 4+ messages in thread
From: Peter Maydell @ 2016-06-13 14:11 UTC (permalink / raw)
  To: QEMU Developers; +Cc: Aurelien Jarno, Leon Alrae, Michael Tokarev

I investigated this qemu-img segfault today
 https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=815409

It's pretty confusing, but as you can see from this gdb log:

0x00513488      185             if (sizef < 0 || sizef > UINT64_MAX) {
2: x/3i $pc
=> 0x513488 <parse_option_size+124>:    move    at,at
   0x51348c <parse_option_size+128>:    bc1f    0x5134fc <parse_option_size+240>
   0x513490 <parse_option_size+132>:    lw      v0,40(sp)
(gdb)
0x0051348c      185             if (sizef < 0 || sizef > UINT64_MAX) {
2: x/3i $pc
=> 0x51348c <parse_option_size+128>:    bc1f    0x5134fc <parse_option_size+240>
   0x513490 <parse_option_size+132>:    lw      v0,40(sp)
   0x513494 <parse_option_size+136>:    lui     v0,0x53
(gdb) info reg
          zero       at       v0       v1       a0       a1       a2       a3
 R0   00000000 00000001 00540000 40f00000 00000000 40f00000 00000000 00000000
            t0       t1       t2       t3       t4       t5       t6       t7
 R8   00000001 00000000 00010000 005280dc 00000001 fffffffc 812299b0 8d5c8000
            s0       s1       s2       s3       s4       s5       s6       s7
 R16  77ff6fbc 0096e764 00526f34 764c8e90 00526f34 00000001 00000000 0098a8b0
            t8       t9       k0       k1       gp       sp       s8       ra
 R24  00000007 76d38400 764ca040 00000000 00571af0 764c8e30 00000000 0051345c
        status       lo       hi badvaddr    cause       pc
      00109cf3 00000000 00000000 764c8ff8 00800024 0051348c
          fcsr      fir  restart
      00000004 00730000 00000000
(gdb) si
warning: GDB can't find the start of the function at 0x764c8e18.

Program received signal SIGSEGV, Segmentation fault.
0x764c8e18 in ?? ()
2: x/3i $pc
=> 0x764c8e18:  lw      v0,40(sp)
   0x764c8e1c:  break   0x202
   0x764c8e20:  tne     zero,zero,0x2f4

when we try to singlestep the bc1f insn we suddenly find ourselves
sat with a segfault at a PC which is slightly below the user SP.

This turns out to be because the in-kernel fp emulation assumes the
stack is always executable; the kernel sources say:
 http://lxr.free-electrons.com/source/arch/mips/math-emu/dsemul.c#L73
"The strategy is to push the instruction onto the user stack and put
 a trap after it which we can catch and jump to the required address"

which is what we've got here; the "BREAK 514" noted in the comment
is the "break 0x202" in the debugger log.

QEMU currently allocates coroutine stacks with a plain g_malloc(),
which makes them r/w but not exec. That's a bug in QEMU which we
should fix (though I'm not sure how best to identify the required
permissions for stacks). It's a bit unhelpful of the kernel to
assume an executable stack and not give a useful diagnostic or
failure mode if it's not true, though.

(You can find a cut down test case for this at:
http://people.linaro.org/~peter.maydell/makecontext.c )

thanks
-- PMM

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] qemu-img segfaults on MIPS hosts due to not having an executable stack
  2016-06-13 14:11 [Qemu-devel] qemu-img segfaults on MIPS hosts due to not having an executable stack Peter Maydell
@ 2016-06-13 14:45 ` Daniel P. Berrange
  2016-06-13 15:16   ` Peter Maydell
  0 siblings, 1 reply; 4+ messages in thread
From: Daniel P. Berrange @ 2016-06-13 14:45 UTC (permalink / raw)
  To: Peter Maydell
  Cc: QEMU Developers, Michael Tokarev, Leon Alrae, Aurelien Jarno

On Mon, Jun 13, 2016 at 03:11:08PM +0100, Peter Maydell wrote:
> I investigated this qemu-img segfault today
>  https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=815409
> 
> It's pretty confusing, but as you can see from this gdb log:
> 
> 0x00513488      185             if (sizef < 0 || sizef > UINT64_MAX) {
> 2: x/3i $pc
> => 0x513488 <parse_option_size+124>:    move    at,at
>    0x51348c <parse_option_size+128>:    bc1f    0x5134fc <parse_option_size+240>
>    0x513490 <parse_option_size+132>:    lw      v0,40(sp)
> (gdb)
> 0x0051348c      185             if (sizef < 0 || sizef > UINT64_MAX) {
> 2: x/3i $pc
> => 0x51348c <parse_option_size+128>:    bc1f    0x5134fc <parse_option_size+240>
>    0x513490 <parse_option_size+132>:    lw      v0,40(sp)
>    0x513494 <parse_option_size+136>:    lui     v0,0x53
> (gdb) info reg
>           zero       at       v0       v1       a0       a1       a2       a3
>  R0   00000000 00000001 00540000 40f00000 00000000 40f00000 00000000 00000000
>             t0       t1       t2       t3       t4       t5       t6       t7
>  R8   00000001 00000000 00010000 005280dc 00000001 fffffffc 812299b0 8d5c8000
>             s0       s1       s2       s3       s4       s5       s6       s7
>  R16  77ff6fbc 0096e764 00526f34 764c8e90 00526f34 00000001 00000000 0098a8b0
>             t8       t9       k0       k1       gp       sp       s8       ra
>  R24  00000007 76d38400 764ca040 00000000 00571af0 764c8e30 00000000 0051345c
>         status       lo       hi badvaddr    cause       pc
>       00109cf3 00000000 00000000 764c8ff8 00800024 0051348c
>           fcsr      fir  restart
>       00000004 00730000 00000000
> (gdb) si
> warning: GDB can't find the start of the function at 0x764c8e18.
> 
> Program received signal SIGSEGV, Segmentation fault.
> 0x764c8e18 in ?? ()
> 2: x/3i $pc
> => 0x764c8e18:  lw      v0,40(sp)
>    0x764c8e1c:  break   0x202
>    0x764c8e20:  tne     zero,zero,0x2f4
> 
> when we try to singlestep the bc1f insn we suddenly find ourselves
> sat with a segfault at a PC which is slightly below the user SP.
> 
> This turns out to be because the in-kernel fp emulation assumes the
> stack is always executable; the kernel sources say:
>  http://lxr.free-electrons.com/source/arch/mips/math-emu/dsemul.c#L73
> "The strategy is to push the instruction onto the user stack and put
>  a trap after it which we can catch and jump to the required address"
> 
> which is what we've got here; the "BREAK 514" noted in the comment
> is the "break 0x202" in the debugger log.
> 
> QEMU currently allocates coroutine stacks with a plain g_malloc(),
> which makes them r/w but not exec. That's a bug in QEMU which we
> should fix (though I'm not sure how best to identify the required
> permissions for stacks). It's a bit unhelpful of the kernel to
> assume an executable stack and not give a useful diagnostic or
> failure mode if it's not true, though.

I'd suggest we just #ifdef the code base on architecture, on that basis
all platforms except mips are probably happy with non-exec stack.

eg, just change the g_malloc(size) to

  mmap(0, size,
  #ifdef (TARGET_MIPS)
       /* FP emulation in linux requires executable stack */
       PROT_READ | PROT_WRITE | PROT_EXEC,
  #else
       PROT_READ | PROT_WRITE,
  #endif
       MAP_PRIVATE | MAP_ANONYMOUS,
       -1, 0);



Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] qemu-img segfaults on MIPS hosts due to not having an executable stack
  2016-06-13 14:45 ` Daniel P. Berrange
@ 2016-06-13 15:16   ` Peter Maydell
  2016-06-13 22:10     ` Ralf Baechle
  0 siblings, 1 reply; 4+ messages in thread
From: Peter Maydell @ 2016-06-13 15:16 UTC (permalink / raw)
  To: Daniel P. Berrange
  Cc: QEMU Developers, Michael Tokarev, Leon Alrae, Aurelien Jarno

On 13 June 2016 at 15:45, Daniel P. Berrange <berrange@redhat.com> wrote:
> On Mon, Jun 13, 2016 at 03:11:08PM +0100, Peter Maydell wrote:
>> QEMU currently allocates coroutine stacks with a plain g_malloc(),
>> which makes them r/w but not exec. That's a bug in QEMU which we
>> should fix (though I'm not sure how best to identify the required
>> permissions for stacks). It's a bit unhelpful of the kernel to
>> assume an executable stack and not give a useful diagnostic or
>> failure mode if it's not true, though.
>
> I'd suggest we just #ifdef the code base on architecture, on that basis
> all platforms except mips are probably happy with non-exec stack.

Have they really all got rid of signal handler trampolines?

(Also, probably breaks running QEMU under QEMU linux-user, because
our linux-user code still assumes trampolines generally :-))

> eg, just change the g_malloc(size) to
>
>   mmap(0, size,
>   #ifdef (TARGET_MIPS)

This wants to be per *host*, not per target.

>        /* FP emulation in linux requires executable stack */
>        PROT_READ | PROT_WRITE | PROT_EXEC,
>   #else
>        PROT_READ | PROT_WRITE,
>   #endif
>        MAP_PRIVATE | MAP_ANONYMOUS,
>        -1, 0);

I have a definite dislike of architecture-ifdef ladders if
we can possibly avoid them. (We have too many already.)

thanks
-- PMM

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] qemu-img segfaults on MIPS hosts due to not having an executable stack
  2016-06-13 15:16   ` Peter Maydell
@ 2016-06-13 22:10     ` Ralf Baechle
  0 siblings, 0 replies; 4+ messages in thread
From: Ralf Baechle @ 2016-06-13 22:10 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Daniel P. Berrange, Leon Alrae, Michael Tokarev, QEMU Developers,
	Aurelien Jarno, James Hogan

On Mon, Jun 13, 2016 at 04:16:02PM +0100, Peter Maydell wrote:

> On 13 June 2016 at 15:45, Daniel P. Berrange <berrange@redhat.com> wrote:
> > On Mon, Jun 13, 2016 at 03:11:08PM +0100, Peter Maydell wrote:
> >> QEMU currently allocates coroutine stacks with a plain g_malloc(),
> >> which makes them r/w but not exec. That's a bug in QEMU which we
> >> should fix (though I'm not sure how best to identify the required
> >> permissions for stacks). It's a bit unhelpful of the kernel to
> >> assume an executable stack and not give a useful diagnostic or
> >> failure mode if it's not true, though.
> >
> > I'd suggest we just #ifdef the code base on architecture, on that basis
> > all platforms except mips are probably happy with non-exec stack.
> 
> Have they really all got rid of signal handler trampolines?

Apparently Android wants a non-executable stack for security reasons.

That said, some special code such as GCC's nested functions may require
stack trampolines.  For such code there is the option to use the p_flags
of the PT_GNU_STACK program header to mark the stack executable.  One
way to do so is to pass the option "-z execstack" to ld or a ".section
.note.GNU-stack,"",@progbits" into the assembler code which is what GCC
will do when generating trampolines.

  Ralf

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2016-06-13 22:10 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-06-13 14:11 [Qemu-devel] qemu-img segfaults on MIPS hosts due to not having an executable stack Peter Maydell
2016-06-13 14:45 ` Daniel P. Berrange
2016-06-13 15:16   ` Peter Maydell
2016-06-13 22:10     ` Ralf Baechle

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).