[LTP] [PATCH v1] getcwd01: Only check buffer NULL on glibc

[LTP] [PATCH v1] getcwd01: Only check buffer NULL on glibc

[Wei Gao via ltp]

Signed-off-by: Wei Gao <wegao@suse.com>
---
 testcases/kernel/syscalls/getcwd/getcwd01.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/testcases/kernel/syscalls/getcwd/getcwd01.c b/testcases/kernel/syscalls/getcwd/getcwd01.c
index 65d827873..6769eb6f4 100644
--- a/testcases/kernel/syscalls/getcwd/getcwd01.c
+++ b/testcases/kernel/syscalls/getcwd/getcwd01.c
@@ -34,10 +34,13 @@ static struct t_case {
 	int exp_err;
 } tcases[] = {
 	{(void *)-1, PATH_MAX, EFAULT},
-	{NULL, (size_t)-1, ENOMEM},
 	{buffer, 0, EINVAL},
 	{buffer, 1, ERANGE},
+/* musl and uclibc-ng will allocate memory before syscall if buf == NULL */
+#ifdef __GLIBC__
+	{NULL, (size_t)-1, ENOMEM},
 	{NULL, 1, ERANGE}
+#endif
 };
 
 static void verify_getcwd(unsigned int n)
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v1] getcwd01: Only check buffer NULL on glibc

[Richard Palethorpe]

Hello,

Wei Gao via ltp <ltp@lists.linux.it> writes:

> Signed-off-by: Wei Gao <wegao@suse.com>
> ---

Need to mention the github issue in the commit
https://github.com/linux-test-project/ltp/issues/1084

>  testcases/kernel/syscalls/getcwd/getcwd01.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/testcases/kernel/syscalls/getcwd/getcwd01.c b/testcases/kernel/syscalls/getcwd/getcwd01.c
> index 65d827873..6769eb6f4 100644
> --- a/testcases/kernel/syscalls/getcwd/getcwd01.c
> +++ b/testcases/kernel/syscalls/getcwd/getcwd01.c
> @@ -34,10 +34,13 @@ static struct t_case {
>  	int exp_err;
>  } tcases[] = {
>  	{(void *)-1, PATH_MAX, EFAULT},
> -	{NULL, (size_t)-1, ENOMEM},
>  	{buffer, 0, EINVAL},
>  	{buffer, 1, ERANGE},
> +/* musl and uclibc-ng will allocate memory before syscall if buf == NULL */
> +#ifdef __GLIBC__
> +	{NULL, (size_t)-1, ENOMEM},

I'd prefer a more general solution. There are a lot of failures like
this. I've seen a lot in the past in glibc where it changed its
behaviour.

Most of these failures follow the same pattern:
pass an invalid argument -> expect a particular kernel error code.

Two things go wrong:
1. libc does something with the invalid value
   a. performs some extended behaviour (like above)
   b. segfaults
2. kernel returns an unexpected error code because the value or context
   are invalid in multiple ways.

For 1. we can usually call the system call directly. This solution has
       been used before. It works for most syscalls except ones like clone.

For 2. Beyond reporting these errors I'm just going to ignore tests like this.

>  	{NULL, 1, ERANGE}
> +#endif
>  };
>  
>  static void verify_getcwd(unsigned int n)
> -- 
> 2.35.3


-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2] getcwd01: Use syscall directly check invalid argument

[Wei Gao via ltp]

Related issue: https://github.com/linux-test-project/ltp/issues/1084

Signed-off-by: Wei Gao <wegao@suse.com>
---
 testcases/kernel/syscalls/getcwd/getcwd01.c | 23 +++++++++++----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/testcases/kernel/syscalls/getcwd/getcwd01.c b/testcases/kernel/syscalls/getcwd/getcwd01.c
index 65d827873..d47174ffc 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,32 @@ 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;
+	int res;
 
 	errno = 0;
-	res = getcwd(tc->buf, tc->size);
-	TST_ERR = errno;
-	if (res) {
+	res = syscall(__NR_getcwd, tc->buf, tc->size);
+	if (!res) {
 		tst_res(TFAIL, "getcwd() succeeded unexpectedly");
 		return;
 	}
 
-	if (TST_ERR != tc->exp_err) {
-		tst_res(TFAIL | TTERRNO, "getcwd() failed unexpectedly, expected %s",
+	if (errno != tc->exp_err) {
+		tst_res(TFAIL, "getcwd() failed unexpectedly, expected %s",
 			tst_strerrno(tc->exp_err));
 		return;
 	}
 
-	tst_res(TPASS | TTERRNO, "getcwd() failed as expected");
+	tst_res(TPASS, "getcwd() failed as expected");
 }
 
 static struct tst_test test = {
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] getcwd01: Use syscall directly check invalid argument

[Petr Vorel]

Hi Wei,

Yes, syscall(__NR_getcwd, ...) is way much better than #ifdef __GLIBC__ in v2.

> Related issue: https://github.com/linux-test-project/ltp/issues/1084
This should be Fixes: #1084


> Signed-off-by: Wei Gao <wegao@suse.com>
> ---
>  testcases/kernel/syscalls/getcwd/getcwd01.c | 23 +++++++++++----------
>  1 file changed, 12 insertions(+), 11 deletions(-)

> diff --git a/testcases/kernel/syscalls/getcwd/getcwd01.c b/testcases/kernel/syscalls/getcwd/getcwd01.c
> index 65d827873..d47174ffc 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,32 @@ 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;
> +	int res;

>  	errno = 0;
> -	res = getcwd(tc->buf, tc->size);
> -	TST_ERR = errno;
> -	if (res) {
> +	res = syscall(__NR_getcwd, tc->buf, tc->size);
> +	if (!res) {
>  		tst_res(TFAIL, "getcwd() succeeded unexpectedly");
>  		return;
>  	}

> -	if (TST_ERR != tc->exp_err) {
> -		tst_res(TFAIL | TTERRNO, "getcwd() failed unexpectedly, expected %s",
> +	if (errno != tc->exp_err) {
> +		tst_res(TFAIL, "getcwd() failed unexpectedly, expected %s",
Hm, macros in tst_test_macros.h does not support char * (getcwd() returns NULL
on failure. I wonder if we want to add TST_EXP_FAIL3(), or maybe macro with a
better name (TST_EXP_FAIL_STR()).

Kind regards,
Petr

>  			tst_strerrno(tc->exp_err));
>  		return;
>  	}

> -	tst_res(TPASS | TTERRNO, "getcwd() failed as expected");
> +	tst_res(TPASS, "getcwd() failed as expected");
>  }

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v3] getcwd01: Use syscall directly check invalid argument

[Wei Gao via ltp]

Fixes: #1084

Signed-off-by: Wei Gao <wegao@suse.com>
---
 testcases/kernel/syscalls/getcwd/getcwd01.c | 27 ++++++---------------
 1 file changed, 7 insertions(+), 20 deletions(-)

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 = {
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] getcwd01: Use syscall directly check invalid argument

[Wei Gao via ltp]

On Thu, Sep 28, 2023 at 07:55:29PM +0200, Petr Vorel wrote:
> Hi Wei,
> 
> Yes, syscall(__NR_getcwd, ...) is way much better than #ifdef __GLIBC__ in v2.
> 
> > Related issue: https://github.com/linux-test-project/ltp/issues/1084
> This should be Fixes: #1084
> 
> 
> > Signed-off-by: Wei Gao <wegao@suse.com>
> > ---
> >  testcases/kernel/syscalls/getcwd/getcwd01.c | 23 +++++++++++----------
> >  1 file changed, 12 insertions(+), 11 deletions(-)
> 
> > diff --git a/testcases/kernel/syscalls/getcwd/getcwd01.c b/testcases/kernel/syscalls/getcwd/getcwd01.c
> > index 65d827873..d47174ffc 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,32 @@ 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;
> > +	int res;
> 
> >  	errno = 0;
> > -	res = getcwd(tc->buf, tc->size);
> > -	TST_ERR = errno;
> > -	if (res) {
> > +	res = syscall(__NR_getcwd, tc->buf, tc->size);
> > +	if (!res) {
> >  		tst_res(TFAIL, "getcwd() succeeded unexpectedly");
> >  		return;
> >  	}
> 
> > -	if (TST_ERR != tc->exp_err) {
> > -		tst_res(TFAIL | TTERRNO, "getcwd() failed unexpectedly, expected %s",
> > +	if (errno != tc->exp_err) {
> > +		tst_res(TFAIL, "getcwd() failed unexpectedly, expected %s",
> Hm, macros in tst_test_macros.h does not support char * (getcwd() returns NULL
> on failure. I wonder if we want to add TST_EXP_FAIL3(), or maybe macro with a
> better name (TST_EXP_FAIL_STR()).
This case we can use TST_EXP_FAIL2 since syscall for __NR_getcwd will return the length of the buffer filled
instead of char *.

Yes, we are missing macros to support char *, i think i can create one in another patch.

> 
> Kind regards,
> Petr
> 
> >  			tst_strerrno(tc->exp_err));
> >  		return;
> >  	}
> 
> > -	tst_res(TPASS | TTERRNO, "getcwd() failed as expected");
> > +	tst_res(TPASS, "getcwd() failed as expected");
> >  }

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3] getcwd01: Use syscall directly check invalid argument

[Petr Vorel]

> Fixes: #1084

You haven't written the reason for switching to raw syscall syscall(__NR_getcwd, ...).
(Adding https://github.com/linux-test-project/ltp/issues/1084 is not enough, the
reason for the change is important enough to write to the commit message).

Also, raw syscall obviously returns a different errnos than glibc wrapper.
But again, it would be better to document in the commit message the reason for
the errno change.



> Signed-off-by: Wei Gao <wegao@suse.com>
> ---
>  testcases/kernel/syscalls/getcwd/getcwd01.c | 27 ++++++---------------
>  1 file changed, 7 insertions(+), 20 deletions(-)

> 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);

If the reason is that getcwd() wrapper in musl is not easily fixable (we
probably don't want #ifdef __GLIBC__), I'm ok for this.

OTOH if test would be easily fixable for musl, we could consider testing both
raw syscall and libc wrapper via .test_variants (separate effort).

Kind regards,
Petr

>  }

>  static struct tst_test test = {

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3] getcwd01: Use syscall directly check invalid argument

[Petr Vorel]

Hi,

> +++ 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.
NOTE: actually before this change we tested getcwd(3), 2 is for raw syscalls, 3
for libc wrappers.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v4] getcwd01: Use syscall directly check invalid argument

[Wei Gao via ltp]

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(-)

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 = {
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3] getcwd01: Use syscall directly check invalid argument

[Wei Gao via ltp]

On Tue, Nov 28, 2023 at 11:52:42AM +0100, Petr Vorel wrote:
> > Fixes: #1084
> 
> You haven't written the reason for switching to raw syscall syscall(__NR_getcwd, ...).
> (Adding https://github.com/linux-test-project/ltp/issues/1084 is not enough, the
> reason for the change is important enough to write to the commit message).
> 
> Also, raw syscall obviously returns a different errnos than glibc wrapper.
> But again, it would be better to document in the commit message the reason for
> the errno change.
I add more comments in patchv4 for improvement.

> > -	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);
> 
> If the reason is that getcwd() wrapper in musl is not easily fixable (we
> probably don't want #ifdef __GLIBC__), I'm ok for this.
> 
> OTOH if test would be easily fixable for musl, we could consider testing both
> raw syscall and libc wrapper via .test_variants (separate effort).
I think we can add extra variant such as LIB_TYPE into t_case and use it to go different check logic.  
But we can not completely avoid use macro to identify musl or not if i understand correctly.
> 
> Kind regards,
> Petr
> 
> >  }
> 
> >  static struct tst_test test = {

Regards
Gao Wei

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v4] getcwd01: Use syscall directly check invalid argument

[Petr Vorel]

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

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v4] getcwd01: Use syscall directly check invalid argument

[Wei Gao via ltp]

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

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp