| Message ID | mvmpm55qnno.fsf@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | linux-user: make sure brk(0) returns a page-aligned value | expand |
On 7/6/23 12:34, Andreas Schwab wrote: > Fixes: 86f04735ac ("linux-user: Fix brk() to release pages") > Signed-off-by: Andreas Schwab <schwab@suse.de> > --- > linux-user/syscall.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/linux-user/syscall.c b/linux-user/syscall.c > index 08162cc966..e8a17377f5 100644 > --- a/linux-user/syscall.c > +++ b/linux-user/syscall.c > @@ -805,7 +805,7 @@ static abi_ulong brk_page; > > void target_set_brk(abi_ulong new_brk) > { > - target_brk = new_brk; > + target_brk = TARGET_PAGE_ALIGN(new_brk); > brk_page = HOST_PAGE_ALIGN(target_brk); > } > It make sense, since that's how do_brk aligns things. I'm curious why this error might have produced host memory clobbering, but I'm not going to debug that. Queuing for tcg/linux-user. r~
On 7/8/23 19:26, Richard Henderson wrote: > On 7/6/23 12:34, Andreas Schwab wrote: >> Fixes: 86f04735ac ("linux-user: Fix brk() to release pages") >> Signed-off-by: Andreas Schwab <schwab@suse.de> >> --- >> linux-user/syscall.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/linux-user/syscall.c b/linux-user/syscall.c >> index 08162cc966..e8a17377f5 100644 >> --- a/linux-user/syscall.c >> +++ b/linux-user/syscall.c >> @@ -805,7 +805,7 @@ static abi_ulong brk_page; >> void target_set_brk(abi_ulong new_brk) >> { >> - target_brk = new_brk; >> + target_brk = TARGET_PAGE_ALIGN(new_brk); >> brk_page = HOST_PAGE_ALIGN(target_brk); >> } > > It make sense, since that's how do_brk aligns things. Yes, patch looks good. I haven't tested, but it seems it adjusts the initial brk(0) value only to make sure that it's target page aligned. Maybe the title should be: ? linux-user: make sure the initial brk(0) is page-aligned > I'm curious why this error might have produced host memory clobbering, but I'm not going to debug that. I don't believe that this un-alignment triggers host memory clobbering either. Helge
On 7/8/23 23:36, Helge Deller wrote: > On 7/8/23 19:26, Richard Henderson wrote: >> On 7/6/23 12:34, Andreas Schwab wrote: >>> Fixes: 86f04735ac ("linux-user: Fix brk() to release pages") >>> Signed-off-by: Andreas Schwab <schwab@suse.de> >>> --- >>> linux-user/syscall.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c >>> index 08162cc966..e8a17377f5 100644 >>> --- a/linux-user/syscall.c >>> +++ b/linux-user/syscall.c >>> @@ -805,7 +805,7 @@ static abi_ulong brk_page; >>> void target_set_brk(abi_ulong new_brk) >>> { >>> - target_brk = new_brk; >>> + target_brk = TARGET_PAGE_ALIGN(new_brk); >>> brk_page = HOST_PAGE_ALIGN(target_brk); >>> } ... >> I'm curious why this error might have produced host memory clobbering, but I'm not going to debug that. > I don't believe that this un-alignment triggers host memory clobbering either. See my follow-up in the other mail threadd: "Re: [RISC-V] ERROR:../accel/tcg/cpu-exec.c:1028:cpu_exec_setjmp: assertion failed: (cpu == current_cpu)" Helge
On 7/8/23 23:36, Helge Deller wrote: > On 7/8/23 19:26, Richard Henderson wrote: >> On 7/6/23 12:34, Andreas Schwab wrote: >>> Fixes: 86f04735ac ("linux-user: Fix brk() to release pages") >>> Signed-off-by: Andreas Schwab <schwab@suse.de> >>> --- >>> linux-user/syscall.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c >>> index 08162cc966..e8a17377f5 100644 >>> --- a/linux-user/syscall.c >>> +++ b/linux-user/syscall.c >>> @@ -805,7 +805,7 @@ static abi_ulong brk_page; >>> void target_set_brk(abi_ulong new_brk) >>> { >>> - target_brk = new_brk; >>> + target_brk = TARGET_PAGE_ALIGN(new_brk); >>> brk_page = HOST_PAGE_ALIGN(target_brk); >>> } >> >> It make sense, since that's how do_brk aligns things. > > Yes, patch looks good. > I haven't tested, but it seems it adjusts the initial brk(0) value > only to make sure that it's target page aligned. > Maybe the title should be: ? > linux-user: make sure the initial brk(0) is page-aligned Another bug report regarding the non-aligned brk()... See: https://github.com/upx/upx/issues/683 https://gitlab.com/qemu-project/qemu/-/issues/1756 Helge
diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 08162cc966..e8a17377f5 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -805,7 +805,7 @@ static abi_ulong brk_page; void target_set_brk(abi_ulong new_brk) { - target_brk = new_brk; + target_brk = TARGET_PAGE_ALIGN(new_brk); brk_page = HOST_PAGE_ALIGN(target_brk); }
Fixes: 86f04735ac ("linux-user: Fix brk() to release pages") Signed-off-by: Andreas Schwab <schwab@suse.de> --- linux-user/syscall.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)