On 18/11/2022 19:39, Teo Couprie Diaz wrote:
Direct usage of brk is discouraged in favor of using malloc. Also, brk was removed from POSIX in POSIX.1-2001. In particular, the musl libc's brk always returns -ENOMEM which causes the LTP tests to prematurely. Invoking the syscall directly allows them to
*exit* prematurely?
properly validate brk behavior.
Instead, use tst_syscall() and handle the failure cases ourselves, as
I think s/Instead,// as you have already introduced that idea in the previous sentence.
we don't depend on the libc to do it anymore.
The patch also removes the dependency on sbrk to get the current break as it has the same issues. Instead, call tst_syscall(__NR_brk, 0) which should always fail, returning the current break.
I think it can't both fail and return the current break :) Probably remove "which should always fail", or maybe s/always/never/?
Aside from that the code looks good to me, and so does patch 2.
Kevin
Signed-off-by: Teo Couprie Diaz teo.coupriediaz@arm.com
testcases/kernel/syscalls/brk/brk01.c | 13 +++++-------- testcases/kernel/syscalls/brk/brk02.c | 14 ++++++-------- 2 files changed, 11 insertions(+), 16 deletions(-)
diff --git a/testcases/kernel/syscalls/brk/brk01.c b/testcases/kernel/syscalls/brk/brk01.c index 5f482b943..d4596c20f 100644 --- a/testcases/kernel/syscalls/brk/brk01.c +++ b/testcases/kernel/syscalls/brk/brk01.c @@ -9,14 +9,15 @@ #include <errno.h> #include "tst_test.h" +#include "lapi/syscalls.h" void verify_brk(void) {
- uintptr_t cur_brk, new_brk;
- void *cur_brk, *new_brk; size_t inc = getpagesize() * 2 - 1; unsigned int i;
- cur_brk = (uintptr_t)sbrk(0);
- cur_brk = (void *)tst_syscall(__NR_brk, 0);
for (i = 0; i < 33; i++) { switch (i % 3) { @@ -31,16 +32,12 @@ void verify_brk(void) break; }
TST_EXP_PASS_SILENT(brk((void *)new_brk), "brk()");if (!TST_PASS)return;cur_brk = (uintptr_t)sbrk(0);
cur_brk = (void *)tst_syscall(__NR_brk, new_brk);if (cur_brk != new_brk) { tst_res(TFAIL, "brk() failed to set address have %p expected %p",
(void *)cur_brk, (void *)new_brk);
cur_brk, new_brk); return;diff --git a/testcases/kernel/syscalls/brk/brk02.c b/testcases/kernel/syscalls/brk/brk02.c index 11e803cb4..dabda30c2 100644 --- a/testcases/kernel/syscalls/brk/brk02.c +++ b/testcases/kernel/syscalls/brk/brk02.c @@ -14,24 +14,22 @@ #include <unistd.h> #include <sys/mman.h> #include "tst_test.h" +#include "lapi/syscalls.h" void brk_down_vmas(void) {
- void *brk_addr = sbrk(0);
- if (brk_addr == (void *) -1)
tst_brk(TBROK, "sbrk() failed");
- void *brk_addr = (void *)tst_syscall(__NR_brk, 0);
unsigned long page_size = getpagesize(); void *addr = brk_addr + page_size;
- if (brk(addr)) {
- if ((void *)tst_syscall(__NR_brk, addr) < addr) { tst_res(TFAIL | TERRNO, "Cannot expand brk() by page size"); return; }
addr += page_size;
- if (brk(addr)) {
- if ((void *)tst_syscall(__NR_brk, addr) < addr) { tst_res(TFAIL | TERRNO, "Cannot expand brk() by 2x page size"); return; }
@@ -42,12 +40,12 @@ void brk_down_vmas(void) } addr += page_size;
- if (brk(addr)) {
- if ((void *)tst_syscall(__NR_brk, addr) < addr) { tst_res(TFAIL | TERRNO, "Cannot expand brk() after mprotect"); return; }
- if (brk(brk_addr)) {
- if ((void *)tst_syscall(__NR_brk, brk_addr) != brk_addr) { tst_res(TFAIL | TERRNO, "Cannot restore brk() to start address"); return; }
base-commit: 75fda47a48d1b6f1c6671fa9819ca9fd9169cf2a