Re: [Qemu-devel] qemu-img segfaults on MIPS hosts due to not having an executable stack

2016-06-13 Thread Ralf Baechle
On Mon, Jun 13, 2016 at 04:16:02PM +0100, Peter Maydell wrote:

> On 13 June 2016 at 15:45, Daniel P. Berrange  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



Re: [Qemu-devel] qemu-img segfaults on MIPS hosts due to not having an executable stack

2016-06-13 Thread Peter Maydell
On 13 June 2016 at 15:45, Daniel P. Berrange  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



Re: [Qemu-devel] qemu-img segfaults on MIPS hosts due to not having an executable stack

2016-06-13 Thread Daniel P. Berrange
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 :moveat,at
>0x51348c :bc1f0x5134fc 
> 
>0x513490 :lw  v0,40(sp)
> (gdb)
> 0x0051348c  185 if (sizef < 0 || sizef > UINT64_MAX) {
> 2: x/3i $pc
> => 0x51348c :bc1f0x5134fc 
> 
>0x513490 :lw  v0,40(sp)
>0x513494 :lui v0,0x53
> (gdb) info reg
>   zero   at   v0   v1   a0   a1   a2   a3
>  R0    0001 0054 40f0  40f0  
> t0   t1   t2   t3   t4   t5   t6   t7
>  R8   0001  0001 005280dc 0001 fffc 812299b0 8d5c8000
> s0   s1   s2   s3   s4   s5   s6   s7
>  R16  77ff6fbc 0096e764 00526f34 764c8e90 00526f34 0001  0098a8b0
> t8   t9   k0   k1   gp   sp   s8   ra
>  R24  0007 76d38400 764ca040  00571af0 764c8e30  0051345c
> status   lo   hi badvaddrcause   pc
>   00109cf3   764c8ff8 00800024 0051348c
>   fcsr  fir  restart
>   0004 0073 
> (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 :|



[Qemu-devel] qemu-img segfaults on MIPS hosts due to not having an executable stack

2016-06-13 Thread Peter Maydell
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 :moveat,at
   0x51348c :bc1f0x5134fc 
   0x513490 :lw  v0,40(sp)
(gdb)
0x0051348c  185 if (sizef < 0 || sizef > UINT64_MAX) {
2: x/3i $pc
=> 0x51348c :bc1f0x5134fc 
   0x513490 :lw  v0,40(sp)
   0x513494 :lui v0,0x53
(gdb) info reg
  zero   at   v0   v1   a0   a1   a2   a3
 R0    0001 0054 40f0  40f0  
t0   t1   t2   t3   t4   t5   t6   t7
 R8   0001  0001 005280dc 0001 fffc 812299b0 8d5c8000
s0   s1   s2   s3   s4   s5   s6   s7
 R16  77ff6fbc 0096e764 00526f34 764c8e90 00526f34 0001  0098a8b0
t8   t9   k0   k1   gp   sp   s8   ra
 R24  0007 76d38400 764ca040  00571af0 764c8e30  0051345c
status   lo   hi badvaddrcause   pc
  00109cf3   764c8ff8 00800024 0051348c
  fcsr  fir  restart
  0004 0073 
(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