| Message ID | 20231201031512.27513-1-wegao@suse.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v4] getcwd01: Use syscall directly check invalid argument | expand |
Hi Wei, > Fixes: #1084 > User space wrap getcwd with different implementation, for example > glibc will directly input parameter into kernel in normal situation > but uclibc-ng and musl will malloc buffer when buffer is NULL, so for > uclibc and musl the parameter size will be ignored. Use system call > directly check invalid argument can be a solution. For the sake of the correctness: there is no malloc() in musl [1] (nor in the mirror source you posted to #1084), that's only in uclibc-ng [2] and glibc [3]. The reason why musl failed was already described by Richie and Cyril in #1084: musl ignores the size parameter when buffer is NULL and allocates it with PATH_MAX and passes this size to kernel. Therefore I reword the commit message. [1] https://git.musl-libc.org/cgit/musl/tree/src/unistd/getcwd.c [2] https://cgit.uclibc-ng.org/cgi/cgit/uclibc-ng.git/tree/libc/sysdeps/linux/common/getcwd.c#n38 [3] https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/getcwd.c;h=5b0b6879ed28f278f07ce494f9be30f504757daa;hb=HEAD#l47 ... > - tst_res(TPASS | TTERRNO, "getcwd() failed as expected"); > + TST_EXP_FAIL2(syscall(__NR_getcwd, tc->buf, tc->size), tc->exp_err); While syscalls() would work everywhere, it's better is LTP wrapper tst_syscall() (it TCONF in case when syscall is not implemented which is I admit nearly impossible). I used that and merged. Thank you! NOTE: we should implement .test_variants, where you just skip affected NULL buffer test (it's enough to test it with raw syscall). Please send a patch or let me know that you don't plan to do it and I'll do it. Kind regards, Petr
On Fri, Dec 01, 2023 at 11:14:15AM +0100, Petr Vorel wrote: > Hi Wei, > > > Fixes: #1084 > > > User space wrap getcwd with different implementation, for example > > glibc will directly input parameter into kernel in normal situation > > but uclibc-ng and musl will malloc buffer when buffer is NULL, so for > > uclibc and musl the parameter size will be ignored. Use system call > > directly check invalid argument can be a solution. > > For the sake of the correctness: there is no malloc() in musl [1] (nor in the > mirror source you posted to #1084), that's only in uclibc-ng [2] and glibc [3]. > > The reason why musl failed was already described by Richie and Cyril in #1084: > musl ignores the size parameter when buffer is NULL and allocates it with PATH_MAX > and passes this size to kernel. > > Therefore I reword the commit message. > > [1] https://git.musl-libc.org/cgit/musl/tree/src/unistd/getcwd.c > [2] https://cgit.uclibc-ng.org/cgi/cgit/uclibc-ng.git/tree/libc/sysdeps/linux/common/getcwd.c#n38 > [3] https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/getcwd.c;h=5b0b6879ed28f278f07ce494f9be30f504757daa;hb=HEAD#l47 > > ... > > - tst_res(TPASS | TTERRNO, "getcwd() failed as expected"); > > + TST_EXP_FAIL2(syscall(__NR_getcwd, tc->buf, tc->size), tc->exp_err); > While syscalls() would work everywhere, it's better is LTP wrapper tst_syscall() > (it TCONF in case when syscall is not implemented which is I admit nearly > impossible). > > I used that and merged. > Thank you! > > NOTE: we should implement .test_variants, where you just skip affected NULL > buffer test (it's enough to test it with raw syscall). Please send a patch or > let me know that you don't plan to do it and I'll do it. I will try to send patch later. Thanks you very much for fix my issue :) Will more carefull next time. > > Kind regards, > Petr
diff --git a/testcases/kernel/syscalls/getcwd/getcwd01.c b/testcases/kernel/syscalls/getcwd/getcwd01.c index 65d827873..ac35383a4 100644 --- a/testcases/kernel/syscalls/getcwd/getcwd01.c +++ b/testcases/kernel/syscalls/getcwd/getcwd01.c @@ -14,8 +14,8 @@ * * Expected Result: * 1) getcwd(2) should return NULL and set errno to EFAULT. - * 2) getcwd(2) should return NULL and set errno to ENOMEM. - * 3) getcwd(2) should return NULL and set errno to EINVAL. + * 2) getcwd(2) should return NULL and set errno to EFAULT. + * 3) getcwd(2) should return NULL and set errno to ERANGE. * 4) getcwd(2) should return NULL and set errno to ERANGE. * 5) getcwd(2) should return NULL and set errno to ERANGE. * @@ -24,6 +24,7 @@ #include <errno.h> #include <unistd.h> #include <limits.h> +#include "lapi/syscalls.h" #include "tst_test.h" static char buffer[5]; @@ -34,32 +35,18 @@ static struct t_case { int exp_err; } tcases[] = { {(void *)-1, PATH_MAX, EFAULT}, - {NULL, (size_t)-1, ENOMEM}, - {buffer, 0, EINVAL}, + {NULL, (size_t)-1, EFAULT}, + {buffer, 0, ERANGE}, {buffer, 1, ERANGE}, {NULL, 1, ERANGE} }; + static void verify_getcwd(unsigned int n) { struct t_case *tc = &tcases[n]; - char *res; - - errno = 0; - res = getcwd(tc->buf, tc->size); - TST_ERR = errno; - if (res) { - tst_res(TFAIL, "getcwd() succeeded unexpectedly"); - return; - } - - if (TST_ERR != tc->exp_err) { - tst_res(TFAIL | TTERRNO, "getcwd() failed unexpectedly, expected %s", - tst_strerrno(tc->exp_err)); - return; - } - tst_res(TPASS | TTERRNO, "getcwd() failed as expected"); + TST_EXP_FAIL2(syscall(__NR_getcwd, tc->buf, tc->size), tc->exp_err); } static struct tst_test test = {
Fixes: #1084 User space wrap getcwd with different implementation, for example glibc will directly input parameter into kernel in normal situation but uclibc-ng and musl will malloc buffer when buffer is NULL, so for uclibc and musl the parameter size will be ignored. Use system call directly check invalid argument can be a solution. Signed-off-by: Wei Gao <wegao@suse.com> --- testcases/kernel/syscalls/getcwd/getcwd01.c | 27 ++++++--------------- 1 file changed, 7 insertions(+), 20 deletions(-)