[PATCH v6 12/13] tests/xetitimer: Use syscalls instead of libc functions

[Dmitry V. Levin]

On Fri, Mar 20, 2020 at 03:09:38PM -0700, Alistair Francis wrote:
> The new glibc implementation for y2038 safe arches (32-bit arch with
> 64-bit time_t) will set the itimerval struct inside libc instead of
> inside the kernel. This is becuase the kernel expects a 32-bit version
> of timeval while libc exposes a 64-bit version. As libc sets this, when
> we pass a faulting pointer we will get a segfault instead of the kernel
> reporint EFAULT back.
> 
> To avoid a seg fault let's use the kernel's equivilent of struct
> __kernel_old_itimerval in the tests and call the syscall directly
> instead of using libc. This way we can trigger the kernels EFAULT
> response correctly.
> 
> Signed-off-by: Alistair Francis <alistair.francis at wdc.com>
> ---
>  tests/xetitimer.c | 36 +++++++++++++++++++++++-------------
>  1 file changed, 23 insertions(+), 13 deletions(-)
> 
> diff --git a/tests/xetitimer.c b/tests/xetitimer.c
> index c49bf87c..e622f59f 100644
> --- a/tests/xetitimer.c
> +++ b/tests/xetitimer.c
> @@ -14,11 +14,17 @@
>  #include <sys/time.h>
>  #include <unistd.h>
>  #include "scno.h"
> +#include "kernel_timeval.h"
> +
> +typedef struct {
> +	kernel_old_timeval_t it_interval;
> +	kernel_old_timeval_t it_value;
> +} kernel_old_itimerval_t;
>  
>  int
>  main(void)
>  {
> -	static const struct itimerval new = {
> +	static const kernel_old_itimerval_t new = {
>  		.it_interval = { 0xc0de1, 0xc0de2 },
>  		.it_value = { 0xc0de3, 0xc0de4 }
>  	};
> @@ -27,12 +33,12 @@ main(void)
>  	static const kernel_ulong_t bogus_timer =
>  		(kernel_ulong_t) 0xfacefeeddeadbeefULL;
>  
> -	TAIL_ALLOC_OBJECT_CONST_PTR(struct itimerval, p_old);
> -	struct itimerval *const p_new = tail_memdup(&new, sizeof(new));
> +	TAIL_ALLOC_OBJECT_CONST_PTR(kernel_old_itimerval_t, p_old);
> +	kernel_old_itimerval_t *const p_new = tail_memdup(&new, sizeof(new));
>  	void *const efault = tail_alloc(sizeof(new) - 8);
>  	long rc;
>  
> -	if (setitimer(ITIMER_REAL, p_new, NULL))
> +	if (syscall(__NR_setitimer, ITIMER_REAL, p_new, NULL))
>  		perror_msg_and_skip("setitimer");
>  	printf("setitimer(ITIMER_REAL"
>  	       ", {it_interval={tv_sec=%lld, tv_usec=%llu}"
> @@ -44,7 +50,7 @@ main(void)
>  	       zero_extend_signed_to_ull(new.it_value.tv_usec));
>  
>  	fill_memory(p_old, sizeof(*p_old));
> -	if (getitimer(ITIMER_REAL, p_old))
> +	if (syscall(__NR_getitimer, ITIMER_REAL, p_old))
>  		perror_msg_and_skip("getitimer");
>  	printf("getitimer(ITIMER_REAL"
>  	       ", {it_interval={tv_sec=%lld, tv_usec=%llu}"
> @@ -55,7 +61,7 @@ main(void)
>  	       zero_extend_signed_to_ull(p_old->it_value.tv_usec));
>  
>  	fill_memory(p_old, sizeof(*p_old));
> -	setitimer(ITIMER_REAL, p_new, p_old);
> +	syscall(__NR_setitimer, ITIMER_REAL, p_new, p_old);
>  	printf("setitimer(ITIMER_REAL"
>  	       ", {it_interval={tv_sec=%lld, tv_usec=%llu}"
>  	       ", it_value={tv_sec=%lld, tv_usec=%llu}}"
> @@ -70,10 +76,10 @@ main(void)
>  	       (long long) p_old->it_value.tv_sec,
>  	       zero_extend_signed_to_ull(p_old->it_value.tv_usec));
>  
> -	rc = getitimer(ITIMER_REAL, efault);
> +	rc = syscall(__NR_getitimer, ITIMER_REAL, efault);
>  	printf("getitimer(ITIMER_REAL, %p) = %s\n", efault, sprintrc(rc));
>  
> -	rc = setitimer(ITIMER_REAL, p_new, efault);
> +	rc = syscall(__NR_setitimer, ITIMER_REAL, p_new, efault);
>  	printf("setitimer(ITIMER_REAL"
>  	       ", {it_interval={tv_sec=%lld, tv_usec=%llu}"
>  	       ", it_value={tv_sec=%lld, tv_usec=%llu}}, %p) = %s\n",
> @@ -83,7 +89,7 @@ main(void)
>  	       zero_extend_signed_to_ull(new.it_value.tv_usec),
>  	       efault, sprintrc(rc));
>  
> -	rc = setitimer(ITIMER_REAL, efault, p_old);
> +	rc = syscall(__NR_setitimer, ITIMER_REAL, efault, p_old);
>  	printf("setitimer(ITIMER_REAL, %p, %p) = %s\n",
>  	       efault, p_old, sprintrc(rc));
>  
> @@ -147,10 +153,14 @@ main(void)
>  
>  	p_new->it_interval.tv_sec = 0xdeadbeefU;
>  	p_new->it_interval.tv_usec = 0xfacefeedU;
> -	p_new->it_value.tv_sec = (time_t) 0xcafef00ddeadbeefLL;
> -	p_new->it_value.tv_usec = (suseconds_t) 0xbadc0dedfacefeedLL;
> -
> -	rc = setitimer(ITIMER_REAL, p_new, p_old);
> +	p_new->it_value.tv_sec = (kernel_long_t) 0xcafef00ddeadbeefLL;
> +# if defined __sparc__ && defined __arch64__
> +	p_new->it_value.tv_usec = (int) 0xbadc0dedfacefeedLL;
> +# else
> +	p_new->it_value.tv_usec = (kernel_long_t) 0xbadc0dedfacefeedLL;
> +# endif

Wouldn't it be better to use typeof instead?


-- 
ldv