Hi, Marc-André Lureau <marcandre.lur...@gmail.com> 于2018年9月12日周三 下午3:16写道:
> Hi > > On Wed, Sep 12, 2018 at 9:22 AM Li Qiang <liq...@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 <liq...@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 >