On 12/12/2022 16:58, Beata Michalska wrote:
Hi Teo,
On Mon, Dec 12, 2022 at 02:59:42PM +0000, Teo Couprie Diaz wrote:
In msync03, sbrk(0) was used and offset to get a pointer to an unmapped area.
How about rewording this a bit with smth along the lines of:
In msync03 testcase, in order to get unmapped memory area, sbrk has been used with an offset making sure the resulting address is beyond the program break.
(or smth like this)
That is much more clear indeed, I will reword along those lines.
This caused issues in purecap because sbrk is now disabled, and
Maybe getting the musl reference in a separate sentence, as currently it somewhat reads bit off:
This causes issues for purecap where brk is being disabled, as well as plain aarch64 due to buggy sbrk implementation in musl (ver. 1.5)
I get what you mean, and it explains the issue better.
in compat with the 1.5 verrsion of musl because of a bug in the sbrk implementation.
Instead, don't rely on sbrk at all and mmap a page, unmap it and use this pointer in order to get an unmapped area of memory, generating an ENOMEM in compat and purecap.
Signed-off-by: Teo Couprie Diaz teo.coupriediaz@arm.com
testcases/kernel/syscalls/msync/msync03.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/testcases/kernel/syscalls/msync/msync03.c b/testcases/kernel/syscalls/msync/msync03.c index 8fa62e57a..6d59c9d26 100644 --- a/testcases/kernel/syscalls/msync/msync03.c +++ b/testcases/kernel/syscalls/msync/msync03.c @@ -130,8 +130,17 @@ static void setup(void) SAFE_GETRLIMIT(NULL, RLIMIT_DATA, &rl); addr3 = (char *)(uintptr_t)rl.rlim_max;
- /* memory pointed to by addr4 was not mapped */
- addr4 = sbrk(0) + (4 * page_sz);
- /* memory pointed to by addr4 is not mapped */
What you could potentially do is to increase the mmap size few lines above and then get addr4 to point to an area that can be safely unmapped - you would avoid extra mmap call, so smth like:
addr1 = SAFE_MMAP(cleanup, 0, page_sz * 2, .... ... addr4 = addr + page_sz; SAFE_MUNMAP(addr4, page_size);
Good point, I didn't think of that ! I'll do some testing just in case but it sounds good.
- addr4 = mmap(NULL, page_sz, PROT_NONE, MAP_PRIVATE, 0, 0);
- if (addr4 == MAP_FAILED) {
addr4 = NULL;- } else {
if (!munmap(addr4, page_sz))tst_resm(TFAIL, "Could not setup test, munmap failed :\n%s",strerror(errno));Otherwise, should the test stop here instead ? It will continue and it will report a failure either way.
I hesitated either way, so if you feel that a failure later is acceptable I'm ok letting it go through here. (If it's needed with the SAFE_MUNMAP, I haven't looked at it yet)
BR B.
Thanks for the review ! Regards, Téo
- }
- tst_resm(TINFO, "addr4 : %p", addr4); }
static void msync_verify(struct test_case_t *tc) -- 2.25.1
linux-morello-ltp mailing list -- linux-morello-ltp@op-lists.linaro.org To unsubscribe send an email to linux-morello-ltp-leave@op-lists.linaro.org