| Message ID | 20190208173520.15007-1-remi@remlab.net |
|---|---|
| State | New |
| Headers | show |
| Series | linux-user: check valid address in access_ok() | expand |
On 08/02/2019 18:35, Rémi Denis-Courmont wrote: > This works around the LTP crash, but there are problably better ways to > go about it. > > Signed-off-by: Rémi Denis-Courmont <remi@remlab.net> > Cc: <lvivier@redhat.com> > --- > linux-user/qemu.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/linux-user/qemu.h b/linux-user/qemu.h > index ef400cb78a..1d222a0cce 100644 > --- a/linux-user/qemu.h > +++ b/linux-user/qemu.h > @@ -457,7 +457,8 @@ extern unsigned long guest_stack_size; > > static inline int access_ok(int type, abi_ulong addr, abi_ulong size) > { > - return page_check_range((target_ulong)addr, size, > + return guest_addr_valid(addr) && guest_addr_valid(addr + size) && I think it should be guest_addr_valid(addr + size - 1). Except that, it looks good. Thanks, Laurent
On 08/02/2019 19:33, Laurent Vivier wrote: > On 08/02/2019 18:35, Rémi Denis-Courmont wrote: >> This works around the LTP crash, but there are problably better ways to >> go about it. >> >> Signed-off-by: Rémi Denis-Courmont <remi@remlab.net> >> Cc: <lvivier@redhat.com> >> --- >> linux-user/qemu.h | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/linux-user/qemu.h b/linux-user/qemu.h >> index ef400cb78a..1d222a0cce 100644 >> --- a/linux-user/qemu.h >> +++ b/linux-user/qemu.h >> @@ -457,7 +457,8 @@ extern unsigned long guest_stack_size; >> >> static inline int access_ok(int type, abi_ulong addr, abi_ulong size) >> { >> - return page_check_range((target_ulong)addr, size, >> + return guest_addr_valid(addr) && guest_addr_valid(addr + size) && > > I think it should be guest_addr_valid(addr + size - 1). In fact (len == 0 || guest_addr_valid(addr + size - 1)). Could you send a new version of your patch? I've received several mail delivery system errors regarding your email address remi@remlab.net. Thanks, Laurent
Hi, I don't think that len == 0 is a sufficient condition to eliminate integer overflow. It only ensures that size - 1 is a positive quantity.
diff --git a/linux-user/qemu.h b/linux-user/qemu.h index ef400cb78a..1d222a0cce 100644 --- a/linux-user/qemu.h +++ b/linux-user/qemu.h @@ -457,7 +457,8 @@ extern unsigned long guest_stack_size; static inline int access_ok(int type, abi_ulong addr, abi_ulong size) { - return page_check_range((target_ulong)addr, size, + return guest_addr_valid(addr) && guest_addr_valid(addr + size) && + page_check_range((target_ulong)addr, size, (type == VERIFY_READ) ? PAGE_READ : (PAGE_READ | PAGE_WRITE)) == 0; }
This works around the LTP crash, but there are problably better ways to go about it. Signed-off-by: Rémi Denis-Courmont <remi@remlab.net> Cc: <lvivier@redhat.com> --- linux-user/qemu.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)