All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrange" <berrange@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: QEMU Developers <qemu-devel@nongnu.org>,
	Michael Tokarev <mjt@tls.msk.ru>,
	Leon Alrae <leon.alrae@imgtec.com>,
	Aurelien Jarno <aurelien@aurel32.net>
Subject: Re: [Qemu-devel] qemu-img segfaults on MIPS hosts due to not having an executable stack
Date: Mon, 13 Jun 2016 15:45:55 +0100	[thread overview]
Message-ID: <20160613144555.GP18794@redhat.com> (raw)
In-Reply-To: <CAFEAcA8nNi1eJzf_CjX6hh+roujhTDnMXLn7isSykzbdCqf--A@mail.gmail.com>

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 :|

  reply	other threads:[~2016-06-13 14:46 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2016-06-13 15:16   ` Peter Maydell
2016-06-13 22:10     ` Ralf Baechle

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160613144555.GP18794@redhat.com \
    --to=berrange@redhat.com \
    --cc=aurelien@aurel32.net \
    --cc=leon.alrae@imgtec.com \
    --cc=mjt@tls.msk.ru \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.