| Message ID | 20200305210534.28392-1-tobias.koch@nonterra.com |
|---|---|
| State | New |
| Headers | show |
| Series | linux-user: mremap fails with EFAULT if address range overlaps with stack guard | expand |
Le 05/03/2020 à 22:05, Tobias Koch a écrit : > If the address range starting at old_address overlaps with the stack guard it > is invalid and mremap must fail with EFAULT. The musl c library relies on this > behavior to detect the stack size, which it does by doing consecutive mremaps > until it hits the stack guard. Without this patch, software (such as the Ruby > interpreter) that calls pthread_getattr_np under musl will crash on 32 bit > targets emulated on a 64 bit host. Could you share some pointers to the code that is doing this? We have already this kind of code in linux-user/elfload.c, setup_arg_pages(): could you check why it doesn't work? Thanks, Laurent
Hi Laurent, the code in musl libc probing the stack is in https://git.musl-libc.org/cgit/musl/plain/src/thread/pthread_getattr_np.c The setup in elfload.c does work, but only when reserved_va is not set. In that case, any stack guard violation is handled by the host kernel and thus results in the expected EFAULT. However, in case of e.g. a 32bit target being emulated on a 64bit host, reserved_va is set and the current code in mmap.c will only produce a more generic ENOMEM, deviating from the kernel's behavior. On 5/7/20 5:35 PM, Laurent Vivier wrote: > Le 05/03/2020 à 22:05, Tobias Koch a écrit : >> If the address range starting at old_address overlaps with the stack guard it >> is invalid and mremap must fail with EFAULT. The musl c library relies on this >> behavior to detect the stack size, which it does by doing consecutive mremaps >> until it hits the stack guard. Without this patch, software (such as the Ruby >> interpreter) that calls pthread_getattr_np under musl will crash on 32 bit >> targets emulated on a 64 bit host. > Could you share some pointers to the code that is doing this? > > We have already this kind of code in linux-user/elfload.c, > setup_arg_pages(): could you check why it doesn't work? > > Thanks, > Laurent
Hm, I see I need to have another look at this :) On 6/15/20 10:17 AM, Tobias Koch wrote: > Hi Laurent, > > the code in musl libc probing the stack is in > > https://git.musl-libc.org/cgit/musl/plain/src/thread/pthread_getattr_np.c > > The setup in elfload.c does work, but only when reserved_va is not set. In that case, any stack guard violation is > handled by the host kernel and thus results in the expected EFAULT. > > However, in case of e.g. a 32bit target being emulated on a 64bit host, reserved_va is set and the current code in > mmap.c will only produce a more generic ENOMEM, deviating from the kernel's behavior. > > > On 5/7/20 5:35 PM, Laurent Vivier wrote: >> Le 05/03/2020 à 22:05, Tobias Koch a écrit : >>> If the address range starting at old_address overlaps with the stack guard it >>> is invalid and mremap must fail with EFAULT. The musl c library relies on this >>> behavior to detect the stack size, which it does by doing consecutive mremaps >>> until it hits the stack guard. Without this patch, software (such as the Ruby >>> interpreter) that calls pthread_getattr_np under musl will crash on 32 bit >>> targets emulated on a 64 bit host. >> Could you share some pointers to the code that is doing this? >> >> We have already this kind of code in linux-user/elfload.c, >> setup_arg_pages(): could you check why it doesn't work? >> >> Thanks, >> Laurent
Ok, so according to the manpage, mremap generates EFAULT when "the range old_address to old_address+old_size is an invalid virtual memory address for this process". This is what the kernel does for the stack guard. However, the mappings in setup_arg_pages() will only ever provoke an ENOMEM, because there is no artifical way to turn a page into an invalid address. So as long as target bits >= host bits, this works as expected and EFAULT is generated, because then mremap is basically passed through and the kernel responds directly. But when reserved_va is set, this needs to be special-cased to fake kernel behavior. I'm open to other suggestions. I also understand that the code duplication in elfload.c and mmap.c to handle this is undesirable, but the most viable alternative seems to be introducing more globals. On 6/15/20 11:28 PM, Tobias Koch wrote: > Hm, I see I need to have another look at this :) > > On 6/15/20 10:17 AM, Tobias Koch wrote: >> Hi Laurent, >> >> the code in musl libc probing the stack is in >> >> https://git.musl-libc.org/cgit/musl/plain/src/thread/pthread_getattr_np.c >> >> The setup in elfload.c does work, but only when reserved_va is not set. In that case, any stack guard violation is >> handled by the host kernel and thus results in the expected EFAULT. >> >> However, in case of e.g. a 32bit target being emulated on a 64bit host, reserved_va is set and the current code in >> mmap.c will only produce a more generic ENOMEM, deviating from the kernel's behavior. >> >> >> On 5/7/20 5:35 PM, Laurent Vivier wrote: >>> Le 05/03/2020 à 22:05, Tobias Koch a écrit : >>>> If the address range starting at old_address overlaps with the stack guard it >>>> is invalid and mremap must fail with EFAULT. The musl c library relies on this >>>> behavior to detect the stack size, which it does by doing consecutive mremaps >>>> until it hits the stack guard. Without this patch, software (such as the Ruby >>>> interpreter) that calls pthread_getattr_np under musl will crash on 32 bit >>>> targets emulated on a 64 bit host. >>> Could you share some pointers to the code that is doing this? >>> >>> We have already this kind of code in linux-user/elfload.c, >>> setup_arg_pages(): could you check why it doesn't work?
Ping On 6/16/20 12:10 AM, Tobias Koch wrote: > Ok, so according to the manpage, mremap generates EFAULT when "the range old_address to old_address+old_size is an > invalid virtual memory address for this process". This is what the kernel does for the stack guard. However, the > mappings in setup_arg_pages() will only ever provoke an ENOMEM, because there is no artifical way to turn a page into an > invalid address. So as long as target bits >= host bits, this works as expected and EFAULT is generated, because then > mremap is basically passed through and the kernel responds directly. But when reserved_va is set, this needs to be > special-cased to fake kernel behavior. > > I'm open to other suggestions. I also understand that the code duplication in elfload.c and mmap.c to handle this is > undesirable, but the most viable alternative seems to be introducing more globals. > > On 6/15/20 11:28 PM, Tobias Koch wrote: >> Hm, I see I need to have another look at this :) >> >> On 6/15/20 10:17 AM, Tobias Koch wrote: >>> Hi Laurent, >>> >>> the code in musl libc probing the stack is in >>> >>> https://git.musl-libc.org/cgit/musl/plain/src/thread/pthread_getattr_np.c >>> >>> The setup in elfload.c does work, but only when reserved_va is not set. In that case, any stack guard violation is >>> handled by the host kernel and thus results in the expected EFAULT. >>> >>> However, in case of e.g. a 32bit target being emulated on a 64bit host, reserved_va is set and the current code in >>> mmap.c will only produce a more generic ENOMEM, deviating from the kernel's behavior. >>> >>> >>> On 5/7/20 5:35 PM, Laurent Vivier wrote: >>>> Le 05/03/2020 à 22:05, Tobias Koch a écrit : >>>>> If the address range starting at old_address overlaps with the stack guard it >>>>> is invalid and mremap must fail with EFAULT. The musl c library relies on this >>>>> behavior to detect the stack size, which it does by doing consecutive mremaps >>>>> until it hits the stack guard. Without this patch, software (such as the Ruby >>>>> interpreter) that calls pthread_getattr_np under musl will crash on 32 bit >>>>> targets emulated on a 64 bit host. >>>> Could you share some pointers to the code that is doing this? >>>> >>>> We have already this kind of code in linux-user/elfload.c, >>>> setup_arg_pages(): could you check why it doesn't work? >
diff --git a/linux-user/mmap.c b/linux-user/mmap.c index 8685f02e7e..62cddbd072 100644 --- a/linux-user/mmap.c +++ b/linux-user/mmap.c @@ -651,6 +651,12 @@ int target_munmap(abi_ulong start, abi_ulong len) return ret; } +#ifdef TARGET_HPPA +#define STACK_GROWS_DOWN 0 +#else +#define STACK_GROWS_DOWN 1 +#endif + abi_long target_mremap(abi_ulong old_addr, abi_ulong old_size, abi_ulong new_size, unsigned long flags, abi_ulong new_addr) @@ -665,6 +671,26 @@ abi_long target_mremap(abi_ulong old_addr, abi_ulong old_size, return -1; } + /* Check that we're not overlapping with the stack guard. */ + if (reserved_va) { + abi_ulong guard_size, guard_start; + + guard_size = TARGET_PAGE_SIZE < qemu_real_host_page_size ? + qemu_real_host_page_size : TARGET_PAGE_SIZE; + + if (STACK_GROWS_DOWN) { + guard_start = reserved_va - guest_stack_size - guard_size; + } else { + guard_start = reserved_va - guard_size; + } + + if (guard_start < old_addr + old_size && + old_addr < guard_start + guard_size) { + errno = EFAULT; + return -1; + } + } + mmap_lock(); if (flags & MREMAP_FIXED) {
If the address range starting at old_address overlaps with the stack guard it is invalid and mremap must fail with EFAULT. The musl c library relies on this behavior to detect the stack size, which it does by doing consecutive mremaps until it hits the stack guard. Without this patch, software (such as the Ruby interpreter) that calls pthread_getattr_np under musl will crash on 32 bit targets emulated on a 64 bit host. Signed-off-by: Tobias Koch <tobias.koch@nonterra.com> --- linux-user/mmap.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)