Re: [Qemu-devel] [PATCH] fw_cfg_mem: add read memory region callback

[Li Qiang <liq3ea@gmail.com>]

Hi,

Marc-André Lureau <marcandre.lureau@gmail.com> 于2018年9月12日周三 下午3:16写道：

> Hi
>
> On Wed, Sep 12, 2018 at 9:22 AM Li Qiang <liq3ea@gmail.com> wrote:
> >
> > The write/read should be paired, this can avoid the
> > NULL-deref while the guest reads the fw_cfg port.
> >
> > Signed-off-by: Li Qiang <liq3ea@gmail.com>
>
> Do you have a reproducer and/or a backtrace?
> memory_region_dispatch_write() checks if ops->write != NULL.
>

As far as I can see, the fw_cfg_mem is not used in x86 and used in arm.
And from my impression, this will NULL read will be a issue. So I just
omit the 'read' field in fw_cfg_comb_mem_ops(this is used for x86) to
emulate the
issue.

When using gdb, I got the following backtrack.

Thread 5 "qemu-system-x86" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffca15b700 (LWP 7637)]
0x0000000000000000 in ?? ()
(gdb) bt
#0  0x0000000000000000 in ?? ()
#1  0x0000555555872492 in memory_region_oldmmio_read_accessor
(mr=0x5555567fa870, addr=1, value=0x7fffca158510, size=1, shift=0,
mask=255, attrs=...) at /home/liqiang02/qemu-devel/qemu/memory.c:409
#2  0x000055555586f2dd in access_with_adjusted_size (addr=addr@entry=1,
value=value@entry=0x7fffca158510, size=size@entry=1,
access_size_min=<optimized out>, access_size_max=<optimized out>,
    access_fn=0x555555872440 <memory_region_oldmmio_read_accessor>,
mr=0x5555567fa870, attrs=...) at
/home/liqiang02/qemu-devel/qemu/memory.c:593
#3  0x0000555555873e90 in memory_region_dispatch_read1 (attrs=..., size=1,
pval=0x7fffca158510, addr=1, mr=0x5555567fa870) at
/home/liqiang02/qemu-devel/qemu/memory.c:1404
#4  memory_region_dispatch_read (mr=mr@entry=0x5555567fa870, addr=1,
pval=pval@entry=0x7fffca158510, size=1, attrs=attrs@entry=...) at
/home/liqiang02/qemu-devel/qemu/memory.c:1423
#5  0x0000555555821e42 in flatview_read_continue (fv=fv@entry=0x7fffbc03f370,
addr=addr@entry=1297, attrs=..., buf=<optimized out>, buf@entry=0x7ffff7fee000
"", len=len@entry=1, addr1=<optimized out>, l=<optimized out>,
mr=0x5555567fa870)
    at /home/liqiang02/qemu-devel/qemu/exec.c:3293
#6  0x0000555555822006 in flatview_read (fv=0x7fffbc03f370, addr=1297,
attrs=..., buf=0x7ffff7fee000 "", len=1) at
/home/liqiang02/qemu-devel/qemu/exec.c:3331
#7  0x000055555582211f in address_space_read_full (as=<optimized out>,
addr=addr@entry=1297, attrs=..., buf=buf@entry=0x7ffff7fee000 "",
len=len@entry=1) at /home/liqiang02/qemu-devel/qemu/exec.c:3344
#8  0x000055555582225a in address_space_rw (as=<optimized out>,
addr=addr@entry=1297, attrs=..., attrs@entry=..., buf=buf@entry=0x7ffff7fee000
"", len=len@entry=1, is_write=is_write@entry=false)
    at /home/liqiang02/qemu-devel/qemu/exec.c:3374
#9  0x0000555555886239 in kvm_handle_io (count=1, size=1,
direction=<optimized out>, data=<optimized out>, attrs=..., port=1297) at
/home/liqiang02/qemu-devel/qemu/accel/kvm/kvm-all.c:1731
#10 kvm_cpu_exec (cpu=cpu@entry=0x5555566e9990) at
/home/liqiang02/qemu-devel/qemu/accel/kvm/kvm-all.c:1971
#11 0x000055555585d3de in qemu_kvm_cpu_thread_fn (arg=0x5555566e9990) at
/home/liqiang02/qemu-devel/qemu/cpus.c:1257
#12 0x00007fffdbd58494 in start_thread () from
/lib/x86_64-linux-gnu/libpthread.so.0
#13 0x00007fffdba9aacf in clone () from /lib/x86_64-linux-gnu/libc.so.6

So I send out this path.

But this time when I not use gdb, there is no segmentation.

So this may not a issue.

If who has a arm environment, he can try this, the PoC is just easy. just
read the 0x510 port with word.

Thanks,
Li Qiang


>
> > ---
> >  hw/nvram/fw_cfg.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> > index d79a568f54..6de7809f1a 100644
> > --- a/hw/nvram/fw_cfg.c
> > +++ b/hw/nvram/fw_cfg.c
> > @@ -434,6 +434,11 @@ static bool fw_cfg_data_mem_valid(void *opaque,
> hwaddr addr,
> >      return addr == 0;
> >  }
> >
> > +static uint64_t fw_cfg_ctl_mem_read(void *opaque, hwaddr addr, unsigned
> size)
> > +{
> > +    return 0;
> > +}
> > +
> >  static void fw_cfg_ctl_mem_write(void *opaque, hwaddr addr,
> >                                   uint64_t value, unsigned size)
> >  {
> > @@ -468,6 +473,7 @@ static bool fw_cfg_comb_valid(void *opaque, hwaddr
> addr,
> >  }
> >
> >  static const MemoryRegionOps fw_cfg_ctl_mem_ops = {
> > +    .read = fw_cfg_ctl_mem_read,
> >      .write = fw_cfg_ctl_mem_write,
> >      .endianness = DEVICE_BIG_ENDIAN,
> >      .valid.accepts = fw_cfg_ctl_mem_valid,
> > --
> > 2.11.0
> >
> >
>
>
> --
> Marc-André Lureau
>