[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] fw_cfg_mem: add read memory region callback
From: |
Li Qiang |
Subject: |
Re: [Qemu-devel] [PATCH] fw_cfg_mem: add read memory region callback |
Date: |
Wed, 12 Sep 2018 16:02:52 +0800 |
Hi,
Marc-André Lureau <address@hidden> 于2018年9月12日周三 下午3:16写道:
> Hi
>
> On Wed, Sep 12, 2018 at 9:22 AM Li Qiang <address@hidden> 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 <address@hidden>
>
> 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 (address@hidden,
address@hidden, address@hidden,
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 (address@hidden, addr=1,
address@hidden, size=1, address@hidden) at
/home/liqiang02/qemu-devel/qemu/memory.c:1423
#5 0x0000555555821e42 in flatview_read_continue (address@hidden,
address@hidden, attrs=..., buf=<optimized out>, address@hidden
"", address@hidden, 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>,
address@hidden, attrs=..., address@hidden "",
address@hidden) at /home/liqiang02/qemu-devel/qemu/exec.c:3344
#8 0x000055555582225a in address_space_rw (as=<optimized out>,
address@hidden, attrs=..., address@hidden, address@hidden
"", address@hidden, address@hidden)
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 (address@hidden) 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
>