memory: check write/read_with_attrs in memory dispatch
PATCH 1/1 memory: check write/read_with_attrs in memory dispatch
This can avoid the NULL-deref if the rm doesn't has a read/write nor write/read_with_attrs callback.
--- memory.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
memory.c +6 -2
@@ -1377,13 +1377,15 @@ static MemTxResult memory_region_dispatch_read1(MemoryRegion *mr, mr->ops->impl.max_access_size, memory_region_read_accessor, mr, attrs); - }else { + }else if (mr->ops->read_with_attrs) {return access_with_adjusted_size (addr, pval, size, mr->ops->impl.min_access_size, mr->ops->impl.max_access_size, memory_region_read_with_attrs_accessor, mr, attrs); } + +return MEMTX_DECODE_ERROR ; } MemTxResult memory_region_dispatch_read(MemoryRegion *mr,
@@ -1454,7 +1456,7 @@ MemTxResult memory_region_dispatch_write(MemoryRegion *mr, mr->ops->impl.max_access_size, memory_region_write_accessor, mr, attrs); - }else { + }else if (mr->ops->write_with_attrs) {return access_with_adjusted_size(addr, &data, size, mr->ops->impl.min_access_size,
@@ -1462,6 +1464,8 @@ MemTxResult memory_region_dispatch_write(MemoryRegion *mr, memory_region_write_with_attrs_accessor, mr, attrs); } + +return MEMTX_DECODE_ERROR ; } void memory_region_init_io(MemoryRegion *mr,
Signature · 1 line
2.11.0
Quoted 7 lines · Li Qiang
This can avoid the NULL-deref if the rm doesn't has a read/write nor write/read_with_attrs callback.
Signed-off-by Li Qiang <[email protected]>--- memory.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
Alternative approach -- assert that every MemoryRegionOps has pointers to callbacks in it, when it is registered in memory_region_init_io() and memory_region_init_rom_device_nomigrate().
I don't have a strong opinion on which is better, but I guess I slightly favour requiring devices to be specific about what their read/write behaviour is.
Do we have many devices that legitimately only want to implement one of read and write, not both ?
thanks
-- PMM
Quoted 4 lines · Peter Maydell <[email protected]> 于2018年11月13日周二 下午5:49写道：
Quoted 3 lines · Li Qiang
This can avoid the NULL-deref if the rm doesn't has a read/write nor write/read_with_attrs callback.
Quoted 9 lines
Quoted 4 lines
Signed-off-by Li Qiang <[email protected]>--- memory.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)Alternative approach -- assert that every MemoryRegionOps has pointers to callbacks in it, when it is registered in
memory_region_init_io()
Actually I have considered this approach, but I rember Paolo remind me that some
of this MR without read callback's is valid because the read can never be called, such as
'notdirty_mem_ops'. So I choose add a check here.
Quoted 8 lines
and
memory_region_init_rom_device_nomigrate().I don't have a strong opinion on which is better, but I guess I slightly favour requiring devices to be specific about what their read/write behaviour is.
Do we have many devices that legitimately only want to implement one of read and write, not both ?
AFAICS, the device has just one read or write is not uncommon. But nearly all of them implement a nop function does nothing. So there just very little lack of the read or write function.
Thanks,
Li Qiang
Quoted 4 lines
thanks
-- PMM
Ping...
It makes sense as when we use 'memory_region_read_accessor' we check mr->ops->read.
but when we use 'memory_region_read_with_attrs_accessor', we doesn't check this.
Thanks,
Li Qiang
Quoted 56 lines · Li Qiang <[email protected]> 于2018年11月13日周二 上午9:42写道：
This can avoid the NULL-deref if the rm doesn't has a read/write nor write/read_with_attrs callback.
Signed-off-by Li Qiang <[email protected]>--- memory.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)memory.c +3
-1@@ -1377,13 +1377,15 @@ static MemTxResultmemory_region_dispatch_read1 (MemoryRegion *mr, mr->ops->impl.max_access_size, memory_region_read_accessor, mr, attrs); - }else { + }else if (mr->ops->read_with_attrs) {return access_with_adjusted_size (addr, pval, size, mr->ops->impl.min_access_size, mr->ops->impl.max_access_size, memory_region_read_with_attrs_accessor, mr, attrs); } + +return MEMTX_DECODE_ERROR ; }MemTxResult memory_region_dispatch_read(MemoryRegion *mr,
memory.c +1
-1@@ -1454,7 +1456,7 @@ MemTxResultmemory_region_dispatch_write (MemoryRegion *mr, mr->ops->impl.max_access_size, memory_region_write_accessor, mr, attrs); - }else { + }else if (mr->ops->write_with_attrs) {return access_with_adjusted_size(addr, &data, size,mr->ops->impl.min_access_size,memory.c +2
-0@@ -1462,6 +1464,8 @@ MemTxResultmemory_region_dispatch_write (MemoryRegion *mr, memory_region_write_with_attrs_accessor, mr, attrs); } + +return MEMTX_DECODE_ERROR ; }void memory_region_init_io(MemoryRegion *mr,
--
2.11.0