[LTP] [PATCH v4] adjtimex02.c: Skipped EFAULT tests for libc variant"

[LTP] [PATCH v4] adjtimex02.c: Skipped EFAULT tests for libc variant"

[Vinay Kumar]

From: vinay kumar <vinay.m.engg@gmail.com>

Tested EFAULT cases only for "__NR_adjtimex" syscall.

Tests for bad addresses in LTP cases trigger segment
fault in libc on a 32bit system.

Signed-off-by: Vinay Kumar <vinay.m.engg@gmail.com>
---
 .../kernel/syscalls/adjtimex/adjtimex02.c     | 226 ++++++++++++------
 1 file changed, 152 insertions(+), 74 deletions(-)

diff --git a/testcases/kernel/syscalls/adjtimex/adjtimex02.c b/testcases/kernel/syscalls/adjtimex/adjtimex02.c
index 19ee97158..5eaea6352 100644
--- a/testcases/kernel/syscalls/adjtimex/adjtimex02.c
+++ b/testcases/kernel/syscalls/adjtimex/adjtimex02.c
@@ -10,115 +10,193 @@
 #include <unistd.h>
 #include <pwd.h>
 #include "tst_test.h"
+#include "lapi/syscalls.h"
 
-#define SET_MODE ( ADJ_OFFSET | ADJ_FREQUENCY | ADJ_MAXERROR | ADJ_ESTERROR | \
-	ADJ_STATUS | ADJ_TIMECONST | ADJ_TICK )
+#define SET_MODE (ADJ_OFFSET | ADJ_FREQUENCY | ADJ_MAXERROR | ADJ_ESTERROR | \
+				ADJ_STATUS | ADJ_TIMECONST | ADJ_TICK)
 
-static int hz;			/* HZ from sysconf */
-
-static struct timex *tim_save;
-static struct timex *buf;
+static int hz;		/* HZ from sysconf */
 
+static struct timex *tim_save, *buf;
 static struct passwd *ltpuser;
 
-static void verify_adjtimex(unsigned int nr)
+struct test_case {
+	unsigned int modes;
+	long lowlimit;
+	long highlimit;
+	long delta;
+	int exp_err;
+};
+
+static int libc_adjtimex(void *timex)
 {
-	struct timex *bufp;
-	int expected_errno = 0;
+	return adjtimex(timex);
+}
 
-	/*
-	 * since Linux 2.6.26, if buf.offset value is outside
-	 * the acceptable range, it is simply normalized instead
-	 * of letting the syscall fail. so just skip this test
-	 * case.
-	 */
-	if (nr > 3 && (tst_kvercmp(2, 6, 25) > 0)) {
-		tst_res(TCONF, "this kernel normalizes buf."
-				"offset value if it is outside"
-				" the acceptable range.");
-		return;
-	}
+static int sys_adjtimex(void *timex)
+{
+	return tst_syscall(__NR_adjtimex, timex);
+}
+
+static struct test_case tc[] = {
+	{
+	.modes = SET_MODE,
+	.exp_err = EFAULT,
+	},
+	{
+	.modes = ADJ_TICK,
+	.lowlimit = 900000,
+	.delta = 1,
+	.exp_err = EINVAL,
+	},
+	{
+	.modes = ADJ_TICK,
+	.highlimit = 1100000,
+	.delta = 1,
+	.exp_err = EINVAL,
+	},
+	{
+	.modes = ADJ_OFFSET,
+	.highlimit = 512000L,
+	.delta = 1,
+	.exp_err = EINVAL,
+	},
+	{
+	.modes = ADJ_OFFSET,
+	.lowlimit = -512000L,
+	.delta = -1,
+	.exp_err = EINVAL,
+	},
+};
+
+static struct test_variants
+{
+	int (*adjtimex)(void *timex);
+	char *desc;
+} variants[] = {
+	{ .adjtimex = libc_adjtimex, .desc = "libc adjtimex()"},
+
+#if (__NR_adjtimex != __LTP__NR_INVALID_SYSCALL)
+	{ .adjtimex = sys_adjtimex,  .desc = "__NR_adjtimex syscall"},
+#endif
+};
+
+static void verify_adjtimex(unsigned int i)
+{
+	struct timex *bufp;
+	struct test_variants *tv = &variants[tst_variant];
 
 	*buf = *tim_save;
 	buf->modes = SET_MODE;
 	bufp = buf;
-	switch (nr) {
-	case 0:
-		bufp = (struct timex *)-1;
-		expected_errno = EFAULT;
-		break;
-	case 1:
-		buf->tick = 900000 / hz - 1;
-		expected_errno = EINVAL;
-		break;
-	case 2:
-		buf->tick = 1100000 / hz + 1;
-		expected_errno = EINVAL;
-		break;
-	case 3:
-		/* Switch to nobody user for correct error code collection */
-		ltpuser = SAFE_GETPWNAM("nobody");
-		SAFE_SETEUID(ltpuser->pw_uid);
-		expected_errno = EPERM;
-		break;
-	case 4:
-		buf->offset = 512000L + 1;
-		expected_errno = EINVAL;
-		break;
-	case 5:
-		buf->offset = (-1) * (512000L) - 1;
-		expected_errno = EINVAL;
-		break;
-	default:
-		tst_brk(TFAIL, "Invalid test case %u ", nr);
+
+	if (tc[i].exp_err == EINVAL) {
+		if (tc[i].modes == ADJ_TICK) {
+			if (tc[i].lowlimit)
+				buf->tick = tc[i].lowlimit - tc[i].delta;
+
+			if (tc[i].highlimit)
+				buf->tick = tc[i].highlimit + tc[i].delta;
+		}
+		if (tc[i].modes == ADJ_OFFSET) {
+			if (tc[i].lowlimit) {
+				tst_res(TCONF, "this kernel normalizes buf. offset value if it is outside the acceptable range.");
+				return;
+			}
+			if (tc[i].highlimit) {
+				tst_res(TCONF, "this kernel normalizes buf. offset value if it is outside the acceptable range.");
+				return;
+			}
+		}
 	}
 
-	TEST(adjtimex(bufp));
-	if ((TST_RET == -1) && (TST_ERR == expected_errno)) {
-		tst_res(TPASS | TTERRNO,
-				"adjtimex() error %u ", expected_errno);
-	} else {
-		tst_res(TFAIL | TTERRNO,
-				"Test Failed, adjtimex() returned %ld",
-				TST_RET);
+	if (tc[i].exp_err == EFAULT) {
+		if (tv->adjtimex != libc_adjtimex) {
+			bufp = (struct timex *) -1;
+		} else {
+			tst_res(TCONF, "EFAULT is skipped for libc variant");
+			return;
+		}
+	}
+
+	TEST(tv->adjtimex(bufp));
+
+	if (tc[i].exp_err != TST_ERR) {
+		tst_res(TFAIL | TTERRNO, "adjtimex(): expected %s mode %x",
+					tst_strerrno(tc[i].exp_err), tc[i].modes);
+		return;
 	}
 
-	/* clean up after ourselves */
-	if (nr == 3)
-		SAFE_SETEUID(0);
+	tst_res(TPASS, "adjtimex() error %s", tst_strerrno(tc[i].exp_err));
+
 }
 
 static void setup(void)
 {
+	struct test_variants *tv = &variants[tst_variant];
+	size_t i;
+	int expected_errno = 0;
+
+	tst_res(TINFO, "Testing variant: %s", tv->desc);
+
 	tim_save->modes = 0;
 
+	buf->modes = SET_MODE;
+
+	/* Switch to nobody user for correct error code collection */
+	ltpuser = SAFE_GETPWNAM("nobody");
+	SAFE_SETEUID(ltpuser->pw_uid);
+	expected_errno = EPERM;
+
+	TEST(tv->adjtimex(buf));
+
+	if ((TST_RET == -1) && (TST_ERR == expected_errno)) {
+		tst_res(TPASS, "adjtimex() error %s ",
+				tst_strerrno(expected_errno));
+	} else {
+		tst_res(TFAIL | TTERRNO,
+				"adjtimex(): returned %ld", TST_RET);
+	}
+
+	SAFE_SETEUID(0);
+
 	/* set the HZ from sysconf */
 	hz = SAFE_SYSCONF(_SC_CLK_TCK);
 
-	/* Save current parameters */
-	if ((adjtimex(tim_save)) == -1)
+	for (i = 0; i < ARRAY_SIZE(tc); i++) {
+		if (tc[i].modes == ADJ_TICK) {
+			tc[i].highlimit /= hz;
+			tc[i].lowlimit /= hz;
+		}
+	}
+
+	if ((adjtimex(tim_save)) == -1) {
 		tst_brk(TBROK | TERRNO,
-			"adjtimex(): failed to save current params");
+		"adjtimex(): failed to save current params");
+	}
 }
 
 static void cleanup(void)
 {
+
 	tim_save->modes = SET_MODE;
 
-	/* Restore saved parameters */
-	if ((adjtimex(tim_save)) == -1)
-		tst_res(TWARN, "Failed to restore saved parameters");
+	if ((adjtimex(tim_save)) == -1) {
+		tst_brk(TBROK | TERRNO,
+		"adjtimex(): failed to save current params");
+	}
 }
 
 static struct tst_test test = {
-	.needs_root = 1,
-	.tcnt = 6,
+	.test = verify_adjtimex,
 	.setup = setup,
 	.cleanup = cleanup,
-	.test = verify_adjtimex,
+	.tcnt = ARRAY_SIZE(tc),
+	.test_variants = ARRAY_SIZE(variants),
+	.needs_root = 1,
 	.bufs = (struct tst_buffers []) {
-		{&buf, .size = sizeof(*buf)},
-		{&tim_save, .size = sizeof(*tim_save)},
-		{},
-	}
+		 {&buf, .size = sizeof(*buf)},
+		 {&tim_save, .size = sizeof(*tim_save)},
+		 {},
+		}
 };
-- 
2.17.1

[LTP] [PATCH v4] adjtimex02.c: Skipped EFAULT tests for libc variant"

[Vinay Kumar]

Hi Metan,

Patch is updated with commit log. Added the reason for EFAULT skipped
for libc variant as below,
"Tests for bad addresses in LTP cases trigger segment fault in libc on
a 32bit system."

Regards,
Vinay

On Thu, Jun 10, 2021 at 10:40 AM Vinay Kumar <vinay.m.engg@gmail.com> wrote:
>
> From: vinay kumar <vinay.m.engg@gmail.com>
>
> Tested EFAULT cases only for "__NR_adjtimex" syscall.
>
> Tests for bad addresses in LTP cases trigger segment
> fault in libc on a 32bit system.
>
> Signed-off-by: Vinay Kumar <vinay.m.engg@gmail.com>
> ---
>  .../kernel/syscalls/adjtimex/adjtimex02.c     | 226 ++++++++++++------
>  1 file changed, 152 insertions(+), 74 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/adjtimex/adjtimex02.c b/testcases/kernel/syscalls/adjtimex/adjtimex02.c
> index 19ee97158..5eaea6352 100644
> --- a/testcases/kernel/syscalls/adjtimex/adjtimex02.c
> +++ b/testcases/kernel/syscalls/adjtimex/adjtimex02.c
> @@ -10,115 +10,193 @@
>  #include <unistd.h>
>  #include <pwd.h>
>  #include "tst_test.h"
> +#include "lapi/syscalls.h"
>
> -#define SET_MODE ( ADJ_OFFSET | ADJ_FREQUENCY | ADJ_MAXERROR | ADJ_ESTERROR | \
> -       ADJ_STATUS | ADJ_TIMECONST | ADJ_TICK )
> +#define SET_MODE (ADJ_OFFSET | ADJ_FREQUENCY | ADJ_MAXERROR | ADJ_ESTERROR | \
> +                               ADJ_STATUS | ADJ_TIMECONST | ADJ_TICK)
>
> -static int hz;                 /* HZ from sysconf */
> -
> -static struct timex *tim_save;
> -static struct timex *buf;
> +static int hz;         /* HZ from sysconf */
>
> +static struct timex *tim_save, *buf;
>  static struct passwd *ltpuser;
>
> -static void verify_adjtimex(unsigned int nr)
> +struct test_case {
> +       unsigned int modes;
> +       long lowlimit;
> +       long highlimit;
> +       long delta;
> +       int exp_err;
> +};
> +
> +static int libc_adjtimex(void *timex)
>  {
> -       struct timex *bufp;
> -       int expected_errno = 0;
> +       return adjtimex(timex);
> +}
>
> -       /*
> -        * since Linux 2.6.26, if buf.offset value is outside
> -        * the acceptable range, it is simply normalized instead
> -        * of letting the syscall fail. so just skip this test
> -        * case.
> -        */
> -       if (nr > 3 && (tst_kvercmp(2, 6, 25) > 0)) {
> -               tst_res(TCONF, "this kernel normalizes buf."
> -                               "offset value if it is outside"
> -                               " the acceptable range.");
> -               return;
> -       }
> +static int sys_adjtimex(void *timex)
> +{
> +       return tst_syscall(__NR_adjtimex, timex);
> +}
> +
> +static struct test_case tc[] = {
> +       {
> +       .modes = SET_MODE,
> +       .exp_err = EFAULT,
> +       },
> +       {
> +       .modes = ADJ_TICK,
> +       .lowlimit = 900000,
> +       .delta = 1,
> +       .exp_err = EINVAL,
> +       },
> +       {
> +       .modes = ADJ_TICK,
> +       .highlimit = 1100000,
> +       .delta = 1,
> +       .exp_err = EINVAL,
> +       },
> +       {
> +       .modes = ADJ_OFFSET,
> +       .highlimit = 512000L,
> +       .delta = 1,
> +       .exp_err = EINVAL,
> +       },
> +       {
> +       .modes = ADJ_OFFSET,
> +       .lowlimit = -512000L,
> +       .delta = -1,
> +       .exp_err = EINVAL,
> +       },
> +};
> +
> +static struct test_variants
> +{
> +       int (*adjtimex)(void *timex);
> +       char *desc;
> +} variants[] = {
> +       { .adjtimex = libc_adjtimex, .desc = "libc adjtimex()"},
> +
> +#if (__NR_adjtimex != __LTP__NR_INVALID_SYSCALL)
> +       { .adjtimex = sys_adjtimex,  .desc = "__NR_adjtimex syscall"},
> +#endif
> +};
> +
> +static void verify_adjtimex(unsigned int i)
> +{
> +       struct timex *bufp;
> +       struct test_variants *tv = &variants[tst_variant];
>
>         *buf = *tim_save;
>         buf->modes = SET_MODE;
>         bufp = buf;
> -       switch (nr) {
> -       case 0:
> -               bufp = (struct timex *)-1;
> -               expected_errno = EFAULT;
> -               break;
> -       case 1:
> -               buf->tick = 900000 / hz - 1;
> -               expected_errno = EINVAL;
> -               break;
> -       case 2:
> -               buf->tick = 1100000 / hz + 1;
> -               expected_errno = EINVAL;
> -               break;
> -       case 3:
> -               /* Switch to nobody user for correct error code collection */
> -               ltpuser = SAFE_GETPWNAM("nobody");
> -               SAFE_SETEUID(ltpuser->pw_uid);
> -               expected_errno = EPERM;
> -               break;
> -       case 4:
> -               buf->offset = 512000L + 1;
> -               expected_errno = EINVAL;
> -               break;
> -       case 5:
> -               buf->offset = (-1) * (512000L) - 1;
> -               expected_errno = EINVAL;
> -               break;
> -       default:
> -               tst_brk(TFAIL, "Invalid test case %u ", nr);
> +
> +       if (tc[i].exp_err == EINVAL) {
> +               if (tc[i].modes == ADJ_TICK) {
> +                       if (tc[i].lowlimit)
> +                               buf->tick = tc[i].lowlimit - tc[i].delta;
> +
> +                       if (tc[i].highlimit)
> +                               buf->tick = tc[i].highlimit + tc[i].delta;
> +               }
> +               if (tc[i].modes == ADJ_OFFSET) {
> +                       if (tc[i].lowlimit) {
> +                               tst_res(TCONF, "this kernel normalizes buf. offset value if it is outside the acceptable range.");
> +                               return;
> +                       }
> +                       if (tc[i].highlimit) {
> +                               tst_res(TCONF, "this kernel normalizes buf. offset value if it is outside the acceptable range.");
> +                               return;
> +                       }
> +               }
>         }
>
> -       TEST(adjtimex(bufp));
> -       if ((TST_RET == -1) && (TST_ERR == expected_errno)) {
> -               tst_res(TPASS | TTERRNO,
> -                               "adjtimex() error %u ", expected_errno);
> -       } else {
> -               tst_res(TFAIL | TTERRNO,
> -                               "Test Failed, adjtimex() returned %ld",
> -                               TST_RET);
> +       if (tc[i].exp_err == EFAULT) {
> +               if (tv->adjtimex != libc_adjtimex) {
> +                       bufp = (struct timex *) -1;
> +               } else {
> +                       tst_res(TCONF, "EFAULT is skipped for libc variant");
> +                       return;
> +               }
> +       }
> +
> +       TEST(tv->adjtimex(bufp));
> +
> +       if (tc[i].exp_err != TST_ERR) {
> +               tst_res(TFAIL | TTERRNO, "adjtimex(): expected %s mode %x",
> +                                       tst_strerrno(tc[i].exp_err), tc[i].modes);
> +               return;
>         }
>
> -       /* clean up after ourselves */
> -       if (nr == 3)
> -               SAFE_SETEUID(0);
> +       tst_res(TPASS, "adjtimex() error %s", tst_strerrno(tc[i].exp_err));
> +
>  }
>
>  static void setup(void)
>  {
> +       struct test_variants *tv = &variants[tst_variant];
> +       size_t i;
> +       int expected_errno = 0;
> +
> +       tst_res(TINFO, "Testing variant: %s", tv->desc);
> +
>         tim_save->modes = 0;
>
> +       buf->modes = SET_MODE;
> +
> +       /* Switch to nobody user for correct error code collection */
> +       ltpuser = SAFE_GETPWNAM("nobody");
> +       SAFE_SETEUID(ltpuser->pw_uid);
> +       expected_errno = EPERM;
> +
> +       TEST(tv->adjtimex(buf));
> +
> +       if ((TST_RET == -1) && (TST_ERR == expected_errno)) {
> +               tst_res(TPASS, "adjtimex() error %s ",
> +                               tst_strerrno(expected_errno));
> +       } else {
> +               tst_res(TFAIL | TTERRNO,
> +                               "adjtimex(): returned %ld", TST_RET);
> +       }
> +
> +       SAFE_SETEUID(0);
> +
>         /* set the HZ from sysconf */
>         hz = SAFE_SYSCONF(_SC_CLK_TCK);
>
> -       /* Save current parameters */
> -       if ((adjtimex(tim_save)) == -1)
> +       for (i = 0; i < ARRAY_SIZE(tc); i++) {
> +               if (tc[i].modes == ADJ_TICK) {
> +                       tc[i].highlimit /= hz;
> +                       tc[i].lowlimit /= hz;
> +               }
> +       }
> +
> +       if ((adjtimex(tim_save)) == -1) {
>                 tst_brk(TBROK | TERRNO,
> -                       "adjtimex(): failed to save current params");
> +               "adjtimex(): failed to save current params");
> +       }
>  }
>
>  static void cleanup(void)
>  {
> +
>         tim_save->modes = SET_MODE;
>
> -       /* Restore saved parameters */
> -       if ((adjtimex(tim_save)) == -1)
> -               tst_res(TWARN, "Failed to restore saved parameters");
> +       if ((adjtimex(tim_save)) == -1) {
> +               tst_brk(TBROK | TERRNO,
> +               "adjtimex(): failed to save current params");
> +       }
>  }
>
>  static struct tst_test test = {
> -       .needs_root = 1,
> -       .tcnt = 6,
> +       .test = verify_adjtimex,
>         .setup = setup,
>         .cleanup = cleanup,
> -       .test = verify_adjtimex,
> +       .tcnt = ARRAY_SIZE(tc),
> +       .test_variants = ARRAY_SIZE(variants),
> +       .needs_root = 1,
>         .bufs = (struct tst_buffers []) {
> -               {&buf, .size = sizeof(*buf)},
> -               {&tim_save, .size = sizeof(*tim_save)},
> -               {},
> -       }
> +                {&buf, .size = sizeof(*buf)},
> +                {&tim_save, .size = sizeof(*tim_save)},
> +                {},
> +               }
>  };
> --
> 2.17.1
>

[LTP] [PATCH v4] adjtimex02.c: Skipped EFAULT tests for libc variant"

[Cyril Hrubis]

Hi!
> Tested EFAULT cases only for "__NR_adjtimex" syscall.
> 
> Tests for bad addresses in LTP cases trigger segment
> fault in libc on a 32bit system.
> 
> Signed-off-by: Vinay Kumar <vinay.m.engg@gmail.com>
> ---
>  .../kernel/syscalls/adjtimex/adjtimex02.c     | 226 ++++++++++++------
>  1 file changed, 152 insertions(+), 74 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/adjtimex/adjtimex02.c b/testcases/kernel/syscalls/adjtimex/adjtimex02.c
> index 19ee97158..5eaea6352 100644
> --- a/testcases/kernel/syscalls/adjtimex/adjtimex02.c
> +++ b/testcases/kernel/syscalls/adjtimex/adjtimex02.c
> @@ -10,115 +10,193 @@
>  #include <unistd.h>
>  #include <pwd.h>
>  #include "tst_test.h"
> +#include "lapi/syscalls.h"
>  
> -#define SET_MODE ( ADJ_OFFSET | ADJ_FREQUENCY | ADJ_MAXERROR | ADJ_ESTERROR | \
> -	ADJ_STATUS | ADJ_TIMECONST | ADJ_TICK )
> +#define SET_MODE (ADJ_OFFSET | ADJ_FREQUENCY | ADJ_MAXERROR | ADJ_ESTERROR | \
> +				ADJ_STATUS | ADJ_TIMECONST | ADJ_TICK)
>  
> -static int hz;			/* HZ from sysconf */
> -
> -static struct timex *tim_save;
> -static struct timex *buf;
> +static int hz;		/* HZ from sysconf */
>  
> +static struct timex *tim_save, *buf;
>  static struct passwd *ltpuser;
>  
> -static void verify_adjtimex(unsigned int nr)
> +struct test_case {
> +	unsigned int modes;
> +	long lowlimit;
> +	long highlimit;
> +	long delta;
> +	int exp_err;
> +};
> +
> +static int libc_adjtimex(void *timex)
>  {
> -	struct timex *bufp;
> -	int expected_errno = 0;
> +	return adjtimex(timex);
> +}

Do we need this indirection?

As long as we fix the prototype in the test_variants to have a proper
type for the function, i.e. the timex pointer is struct timex * instead
of void * we can store the libc function pointer directly to the variant
structure.

> -	/*
> -	 * since Linux 2.6.26, if buf.offset value is outside
> -	 * the acceptable range, it is simply normalized instead
> -	 * of letting the syscall fail. so just skip this test
> -	 * case.
> -	 */
> -	if (nr > 3 && (tst_kvercmp(2, 6, 25) > 0)) {
> -		tst_res(TCONF, "this kernel normalizes buf."
> -				"offset value if it is outside"
> -				" the acceptable range.");
> -		return;
> -	}
> +static int sys_adjtimex(void *timex)
                            ^
			    This should be struct timex *
> +{
> +	return tst_syscall(__NR_adjtimex, timex);
> +}
> +
> +static struct test_case tc[] = {
> +	{
> +	.modes = SET_MODE,
> +	.exp_err = EFAULT,
> +	},
> +	{
> +	.modes = ADJ_TICK,
> +	.lowlimit = 900000,
> +	.delta = 1,
> +	.exp_err = EINVAL,
> +	},
> +	{
> +	.modes = ADJ_TICK,
> +	.highlimit = 1100000,
> +	.delta = 1,
> +	.exp_err = EINVAL,
> +	},
> +	{
> +	.modes = ADJ_OFFSET,
> +	.highlimit = 512000L,
> +	.delta = 1,
> +	.exp_err = EINVAL,
> +	},
> +	{
> +	.modes = ADJ_OFFSET,
> +	.lowlimit = -512000L,
> +	.delta = -1,
> +	.exp_err = EINVAL,
> +	},
> +};
> +
> +static struct test_variants
> +{
> +	int (*adjtimex)(void *timex);
                        ^
			This should be struct timex * as well.
> +	char *desc;
> +} variants[] = {
> +	{ .adjtimex = libc_adjtimex, .desc = "libc adjtimex()"},
> +
> +#if (__NR_adjtimex != __LTP__NR_INVALID_SYSCALL)
> +	{ .adjtimex = sys_adjtimex,  .desc = "__NR_adjtimex syscall"},
> +#endif
> +};
> +
> +static void verify_adjtimex(unsigned int i)
> +{
> +	struct timex *bufp;
> +	struct test_variants *tv = &variants[tst_variant];
>  
>  	*buf = *tim_save;
>  	buf->modes = SET_MODE;
>  	bufp = buf;
> -	switch (nr) {
> -	case 0:
> -		bufp = (struct timex *)-1;
> -		expected_errno = EFAULT;
> -		break;
> -	case 1:
> -		buf->tick = 900000 / hz - 1;
> -		expected_errno = EINVAL;
> -		break;
> -	case 2:
> -		buf->tick = 1100000 / hz + 1;
> -		expected_errno = EINVAL;
> -		break;
> -	case 3:
> -		/* Switch to nobody user for correct error code collection */
> -		ltpuser = SAFE_GETPWNAM("nobody");
> -		SAFE_SETEUID(ltpuser->pw_uid);
> -		expected_errno = EPERM;
> -		break;
> -	case 4:
> -		buf->offset = 512000L + 1;
> -		expected_errno = EINVAL;
> -		break;
> -	case 5:
> -		buf->offset = (-1) * (512000L) - 1;
> -		expected_errno = EINVAL;
> -		break;
> -	default:
> -		tst_brk(TFAIL, "Invalid test case %u ", nr);
> +
> +	if (tc[i].exp_err == EINVAL) {
> +		if (tc[i].modes == ADJ_TICK) {
> +			if (tc[i].lowlimit)
> +				buf->tick = tc[i].lowlimit - tc[i].delta;
> +
> +			if (tc[i].highlimit)
> +				buf->tick = tc[i].highlimit + tc[i].delta;
> +		}
> +		if (tc[i].modes == ADJ_OFFSET) {
> +			if (tc[i].lowlimit) {
> +				tst_res(TCONF, "this kernel normalizes buf. offset value if it is outside the acceptable range.");
> +				return;
> +			}
> +			if (tc[i].highlimit) {
> +				tst_res(TCONF, "this kernel normalizes buf. offset value if it is outside the acceptable range.");
> +				return;
> +			}
> +		}
>  	}
>  
> -	TEST(adjtimex(bufp));
> -	if ((TST_RET == -1) && (TST_ERR == expected_errno)) {
> -		tst_res(TPASS | TTERRNO,
> -				"adjtimex() error %u ", expected_errno);
> -	} else {
> -		tst_res(TFAIL | TTERRNO,
> -				"Test Failed, adjtimex() returned %ld",
> -				TST_RET);
> +	if (tc[i].exp_err == EFAULT) {
> +		if (tv->adjtimex != libc_adjtimex) {
> +			bufp = (struct timex *) -1;
> +		} else {
> +			tst_res(TCONF, "EFAULT is skipped for libc variant");
> +			return;
> +		}
> +	}
> +
> +	TEST(tv->adjtimex(bufp));
> +
> +	if (tc[i].exp_err != TST_ERR) {
> +		tst_res(TFAIL | TTERRNO, "adjtimex(): expected %s mode %x",
> +					tst_strerrno(tc[i].exp_err), tc[i].modes);
> +		return;
>  	}

We should really use TST_EXP_FAIL() here instead.

> -	/* clean up after ourselves */
> -	if (nr == 3)
> -		SAFE_SETEUID(0);
> +	tst_res(TPASS, "adjtimex() error %s", tst_strerrno(tc[i].exp_err));
> +
>  }
>  
>  static void setup(void)
>  {
> +	struct test_variants *tv = &variants[tst_variant];
> +	size_t i;
> +	int expected_errno = 0;
> +
> +	tst_res(TINFO, "Testing variant: %s", tv->desc);
> +
>  	tim_save->modes = 0;
>  
> +	buf->modes = SET_MODE;
> +
> +	/* Switch to nobody user for correct error code collection */
> +	ltpuser = SAFE_GETPWNAM("nobody");
> +	SAFE_SETEUID(ltpuser->pw_uid);
> +	expected_errno = EPERM;
> +
> +	TEST(tv->adjtimex(buf));
> +
> +	if ((TST_RET == -1) && (TST_ERR == expected_errno)) {
> +		tst_res(TPASS, "adjtimex() error %s ",
> +				tst_strerrno(expected_errno));
> +	} else {
> +		tst_res(TFAIL | TTERRNO,
> +				"adjtimex(): returned %ld", TST_RET);
> +	}
> +
> +	SAFE_SETEUID(0);

What is this nonsense?

What is doing a test code in the test setup?

The only thing that should be done in the setup is to store the nobody
uid into a global variable.

The EPERM test should be done in the verify_adjtimex() function, we can
do something as:

	if (tc[i].exp_err == EPERM)
		SAFE_SETEUID(nobody_uid);


	/* do the actuall test */

	if (tc[i].exp_err == EPERM)
		SAFE_SETEUID(0);

>  	/* set the HZ from sysconf */
>  	hz = SAFE_SYSCONF(_SC_CLK_TCK);
>  
> -	/* Save current parameters */
> -	if ((adjtimex(tim_save)) == -1)
> +	for (i = 0; i < ARRAY_SIZE(tc); i++) {
> +		if (tc[i].modes == ADJ_TICK) {
> +			tc[i].highlimit /= hz;
> +			tc[i].lowlimit /= hz;
> +		}
> +	}
> +
> +	if ((adjtimex(tim_save)) == -1) {
>  		tst_brk(TBROK | TERRNO,
> -			"adjtimex(): failed to save current params");
> +		"adjtimex(): failed to save current params");
> +	}
>  }
>  
>  static void cleanup(void)
>  {
> +
>  	tim_save->modes = SET_MODE;
>  
> -	/* Restore saved parameters */
> -	if ((adjtimex(tim_save)) == -1)
> -		tst_res(TWARN, "Failed to restore saved parameters");
> +	if ((adjtimex(tim_save)) == -1) {
> +		tst_brk(TBROK | TERRNO,
> +		"adjtimex(): failed to save current params");
                                  ^
				  This was correct before the change, we
				  change the modes to SET_MODE so we
				  really restore the value here.
> +	}
>  }
>  
>  static struct tst_test test = {
> -	.needs_root = 1,
> -	.tcnt = 6,
> +	.test = verify_adjtimex,
>  	.setup = setup,
>  	.cleanup = cleanup,
> -	.test = verify_adjtimex,
> +	.tcnt = ARRAY_SIZE(tc),
> +	.test_variants = ARRAY_SIZE(variants),
> +	.needs_root = 1,
>  	.bufs = (struct tst_buffers []) {
> -		{&buf, .size = sizeof(*buf)},
> -		{&tim_save, .size = sizeof(*tim_save)},
> -		{},
> -	}
> +		 {&buf, .size = sizeof(*buf)},
> +		 {&tim_save, .size = sizeof(*tim_save)},
> +		 {},
> +		}
>  };
> -- 
> 2.17.1
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v5] adjtimex02.c: Skipped EFAULT tests for libc variant

[Vinay Kumar]

Tested EFAULT cases only for "__NR_adjtimex" syscall.

Tests for bad addresses in LTP cases trigger segment
fault in libc on a 32bit system.

Signed-off-by: Vinay Kumar <vinay.m.engg@gmail.com>
---
 .../kernel/syscalls/adjtimex/adjtimex02.c     | 169 +++++++++++-------
 1 file changed, 101 insertions(+), 68 deletions(-)

diff --git a/testcases/kernel/syscalls/adjtimex/adjtimex02.c b/testcases/kernel/syscalls/adjtimex/adjtimex02.c
index 19ee97158..bfe07ec72 100644
--- a/testcases/kernel/syscalls/adjtimex/adjtimex02.c
+++ b/testcases/kernel/syscalls/adjtimex/adjtimex02.c
@@ -10,99 +10,131 @@
 #include <unistd.h>
 #include <pwd.h>
 #include "tst_test.h"
+#include "lapi/syscalls.h"
 
-#define SET_MODE ( ADJ_OFFSET | ADJ_FREQUENCY | ADJ_MAXERROR | ADJ_ESTERROR | \
-	ADJ_STATUS | ADJ_TIMECONST | ADJ_TICK )
+#define SET_MODE (ADJ_OFFSET | ADJ_FREQUENCY | ADJ_MAXERROR | ADJ_ESTERROR | \
+				ADJ_STATUS | ADJ_TIMECONST | ADJ_TICK)
 
-static int hz;			/* HZ from sysconf */
-
-static struct timex *tim_save;
-static struct timex *buf;
+static int hz;		/* HZ from sysconf */
 
+static struct timex *tim_save, *buf;
 static struct passwd *ltpuser;
 
-static void verify_adjtimex(unsigned int nr)
+static int libc_adjtimex(struct timex *value)
+{
+	return adjtimex(value);
+}
+
+static int sys_adjtimex(struct timex *value)
+{
+	return tst_syscall(__NR_adjtimex, value);
+}
+
+static struct test_case {
+	unsigned int modes;
+	long lowlimit;
+	long highlimit;
+	long delta;
+	int exp_err;
+} tc[] = {
+	{ .modes = SET_MODE, .exp_err = EPERM},
+	{ .modes = SET_MODE, .exp_err = EFAULT},
+	{ .modes = ADJ_TICK, .lowlimit = 900000, .delta = 1, .exp_err = EINVAL},
+	{ .modes = ADJ_TICK, .highlimit = 1100000, .delta = 1, .exp_err = EINVAL},
+	{ .modes = ADJ_OFFSET, .highlimit = 512000L, .delta = 1, .exp_err = EINVAL},
+	{ .modes = ADJ_OFFSET, .lowlimit = -512000L, .delta = -1,	.exp_err = EINVAL},
+};
+
+static struct test_variants
+{
+	int (*adjtimex)(struct timex *value);
+	char *desc;
+} variants[] = {
+	{ .adjtimex = libc_adjtimex, .desc = "libc adjtimex()"},
+
+#if (__NR_adjtimex != __LTP__NR_INVALID_SYSCALL)
+	{ .adjtimex = sys_adjtimex,  .desc = "__NR_adjtimex syscall"},
+#endif
+};
+
+static void verify_adjtimex(unsigned int i)
 {
 	struct timex *bufp;
-	int expected_errno = 0;
-
-	/*
-	 * since Linux 2.6.26, if buf.offset value is outside
-	 * the acceptable range, it is simply normalized instead
-	 * of letting the syscall fail. so just skip this test
-	 * case.
-	 */
-	if (nr > 3 && (tst_kvercmp(2, 6, 25) > 0)) {
-		tst_res(TCONF, "this kernel normalizes buf."
-				"offset value if it is outside"
-				" the acceptable range.");
-		return;
-	}
+	struct test_variants *tv = &variants[tst_variant];
 
 	*buf = *tim_save;
 	buf->modes = SET_MODE;
 	bufp = buf;
-	switch (nr) {
-	case 0:
-		bufp = (struct timex *)-1;
-		expected_errno = EFAULT;
-		break;
-	case 1:
-		buf->tick = 900000 / hz - 1;
-		expected_errno = EINVAL;
-		break;
-	case 2:
-		buf->tick = 1100000 / hz + 1;
-		expected_errno = EINVAL;
-		break;
-	case 3:
-		/* Switch to nobody user for correct error code collection */
-		ltpuser = SAFE_GETPWNAM("nobody");
+
+	if (tc[i].exp_err == EPERM)
 		SAFE_SETEUID(ltpuser->pw_uid);
-		expected_errno = EPERM;
-		break;
-	case 4:
-		buf->offset = 512000L + 1;
-		expected_errno = EINVAL;
-		break;
-	case 5:
-		buf->offset = (-1) * (512000L) - 1;
-		expected_errno = EINVAL;
-		break;
-	default:
-		tst_brk(TFAIL, "Invalid test case %u ", nr);
+
+	if (tc[i].exp_err == EINVAL) {
+		if (tc[i].modes == ADJ_TICK) {
+			if (tc[i].lowlimit)
+				buf->tick = tc[i].lowlimit - tc[i].delta;
+
+			if (tc[i].highlimit)
+				buf->tick = tc[i].highlimit + tc[i].delta;
+		}
+		if (tc[i].modes == ADJ_OFFSET) {
+			if (tc[i].lowlimit) {
+				tst_res(TCONF, "this kernel normalizes buf. offset value if it is outside the acceptable range.");
+				return;
+			}
+			if (tc[i].highlimit) {
+				tst_res(TCONF, "this kernel normalizes buf. offset value if it is outside the acceptable range.");
+				return;
+			}
+		}
 	}
 
-	TEST(adjtimex(bufp));
-	if ((TST_RET == -1) && (TST_ERR == expected_errno)) {
-		tst_res(TPASS | TTERRNO,
-				"adjtimex() error %u ", expected_errno);
-	} else {
-		tst_res(TFAIL | TTERRNO,
-				"Test Failed, adjtimex() returned %ld",
-				TST_RET);
+	if (tc[i].exp_err == EFAULT) {
+		if (tv->adjtimex != libc_adjtimex) {
+			bufp = (struct timex *) -1;
+		} else {
+			tst_res(TCONF, "EFAULT is skipped for libc variant");
+			return;
+		}
 	}
 
-	/* clean up after ourselves */
-	if (nr == 3)
+	TST_EXP_FAIL(tv->adjtimex(bufp), tc[i].exp_err, "adjtimex() error");
+
+	if (tc[i].exp_err == EPERM)
 		SAFE_SETEUID(0);
 }
 
 static void setup(void)
 {
+	struct test_variants *tv = &variants[tst_variant];
+	size_t i;
+
+	tst_res(TINFO, "Testing variant: %s", tv->desc);
+
 	tim_save->modes = 0;
 
+	ltpuser = SAFE_GETPWNAM("nobody");
+	SAFE_SETEUID(ltpuser->pw_uid);
+
 	/* set the HZ from sysconf */
 	hz = SAFE_SYSCONF(_SC_CLK_TCK);
 
-	/* Save current parameters */
-	if ((adjtimex(tim_save)) == -1)
+	for (i = 0; i < ARRAY_SIZE(tc); i++) {
+		if (tc[i].modes == ADJ_TICK) {
+			tc[i].highlimit /= hz;
+			tc[i].lowlimit /= hz;
+		}
+	}
+
+	if ((adjtimex(tim_save)) == -1) {
 		tst_brk(TBROK | TERRNO,
 			"adjtimex(): failed to save current params");
+	}
 }
 
 static void cleanup(void)
 {
+
 	tim_save->modes = SET_MODE;
 
 	/* Restore saved parameters */
@@ -111,14 +143,15 @@ static void cleanup(void)
 }
 
 static struct tst_test test = {
-	.needs_root = 1,
-	.tcnt = 6,
+	.test = verify_adjtimex,
 	.setup = setup,
 	.cleanup = cleanup,
-	.test = verify_adjtimex,
+	.tcnt = ARRAY_SIZE(tc),
+	.test_variants = ARRAY_SIZE(variants),
+	.needs_root = 1,
 	.bufs = (struct tst_buffers []) {
-		{&buf, .size = sizeof(*buf)},
-		{&tim_save, .size = sizeof(*tim_save)},
-		{},
-	}
+		 {&buf, .size = sizeof(*buf)},
+		 {&tim_save, .size = sizeof(*tim_save)},
+		 {},
+		}
 };
-- 
2.17.1

[LTP] [PATCH v5] adjtimex02.c: Skipped EFAULT tests for libc variant

[Cyril Hrubis]

Hi!
Pushed with changes, thanks.

* Removed a few useless whitespace changes

* Added back the kernel version check, without that there would be no
  point in keeping the ADJ_OFFSET tests, and maybe these should have
  been removed now since 2.6.26 is prehistoric

* Added docparse formatted comment

* Changed the buf->modes to use the tc[i].modes instead of SET_MODES

-- 
Cyril Hrubis
chrubis@suse.cz