Re: [PATCH v3] linux-user: Fix 'semop()' and 'semtimedop()' implementation
Le 18/08/2020 à 20:07, Filip Bozuta a écrit :
> The implementations of syscalls 'semop()' and 'semtimedop()' in
> file 'syscall.c' use function 'target_to_host_sembuf()' to convert
> values of 'struct sembuf' from host to target. However, before this
> conversion it should be check whether the number of semaphore operations
> 'nsops' is not bigger than maximum allowed semaphor operations per
> syscall: 'SEMOPM'. In these cases, errno 'E2BIG' ("Arg list too long")
> should be set. But the implementation will set errno 'EFAULT' ("Bad address")
> in this case since the conversion from target to host in this case fails.
>
> This was confirmed with the LTP test for 'semop()' ('ipc/semop/semop02') in
> test case where 'nsops' is greater than SEMOPM with unaproppriate errno
> EFAULT:
>
> semop02.c:130: FAIL: semop failed unexpectedly; expected: E2BIG: EFAULT (14)
>
> This patch changes this by adding a check whether 'nsops' is bigger than
> 'SEMOPM' before the conversion function 'target_to_host_sembuf()' is called.
> After the changes from this patch, the test works fine along with the other
> LTP testcases for 'semop()'):
>
> semop02.c:126: PASS: semop failed as expected: E2BIG (7)
>
> Implementation notes:
>
> A target value ('TARGET_SEMOPM') was added for 'SEMOPM' as to be sure
> in case the value is not available for some targets.
>
> Signed-off-by: Filip Bozuta
> ---
> linux-user/syscall.c | 13 +++--
> linux-user/syscall_defs.h | 2 ++
> 2 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 1211e759c2..e4d12c29d3 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -3904,7 +3904,7 @@ static inline abi_long do_semtimedop(int semid,
> unsigned nsops,
> abi_long timeout)
> {
> -struct sembuf sops[nsops];
> +struct sembuf *sops;
> struct timespec ts, *pts = NULL;
> abi_long ret;
>
> @@ -3915,8 +3915,16 @@ static inline abi_long do_semtimedop(int semid,
> }
> }
>
> -if (target_to_host_sembuf(sops, ptr, nsops))
> +if (nsops > TARGET_SEMOPM) {
> +return -TARGET_E2BIG;
> +}
> +
> +sops = g_new(struct sembuf, nsops);
> +
> +if (target_to_host_sembuf(sops, ptr, nsops)) {
> +g_free(sops);
> return -TARGET_EFAULT;
> +}
>
> ret = -TARGET_ENOSYS;
> #ifdef __NR_semtimedop
> @@ -3928,6 +3936,7 @@ static inline abi_long do_semtimedop(int semid,
> SEMTIMEDOP_IPC_ARGS(nsops, sops,
> (long)pts)));
> }
> #endif
> +g_free(sops);
> return ret;
> }
> #endif
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index 3c261cff0e..f7f77346be 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -46,6 +46,8 @@
> #define IPCOP_shmget 23
> #define IPCOP_shmctl 24
>
> +#define TARGET_SEMOPM 500
> +
> /*
> * The following is for compatibility across the various Linux
> * platforms. The i386 ioctl numbering scheme doesn't really enforce
>
Applied to my linux-user-for-5.2 branch.
Thanks,
Laurent
Re: [PATCH v3] linux-user: Fix 'semop()' and 'semtimedop()' implementation
Le 18/08/2020 à 20:07, Filip Bozuta a écrit :
> The implementations of syscalls 'semop()' and 'semtimedop()' in
> file 'syscall.c' use function 'target_to_host_sembuf()' to convert
> values of 'struct sembuf' from host to target. However, before this
> conversion it should be check whether the number of semaphore operations
> 'nsops' is not bigger than maximum allowed semaphor operations per
> syscall: 'SEMOPM'. In these cases, errno 'E2BIG' ("Arg list too long")
> should be set. But the implementation will set errno 'EFAULT' ("Bad address")
> in this case since the conversion from target to host in this case fails.
>
> This was confirmed with the LTP test for 'semop()' ('ipc/semop/semop02') in
> test case where 'nsops' is greater than SEMOPM with unaproppriate errno
> EFAULT:
>
> semop02.c:130: FAIL: semop failed unexpectedly; expected: E2BIG: EFAULT (14)
>
> This patch changes this by adding a check whether 'nsops' is bigger than
> 'SEMOPM' before the conversion function 'target_to_host_sembuf()' is called.
> After the changes from this patch, the test works fine along with the other
> LTP testcases for 'semop()'):
>
> semop02.c:126: PASS: semop failed as expected: E2BIG (7)
>
> Implementation notes:
>
> A target value ('TARGET_SEMOPM') was added for 'SEMOPM' as to be sure
> in case the value is not available for some targets.
>
> Signed-off-by: Filip Bozuta
> ---
> linux-user/syscall.c | 13 +++--
> linux-user/syscall_defs.h | 2 ++
> 2 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 1211e759c2..e4d12c29d3 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -3904,7 +3904,7 @@ static inline abi_long do_semtimedop(int semid,
> unsigned nsops,
> abi_long timeout)
> {
> -struct sembuf sops[nsops];
> +struct sembuf *sops;
> struct timespec ts, *pts = NULL;
> abi_long ret;
>
> @@ -3915,8 +3915,16 @@ static inline abi_long do_semtimedop(int semid,
> }
> }
>
> -if (target_to_host_sembuf(sops, ptr, nsops))
> +if (nsops > TARGET_SEMOPM) {
> +return -TARGET_E2BIG;
> +}
> +
> +sops = g_new(struct sembuf, nsops);
> +
> +if (target_to_host_sembuf(sops, ptr, nsops)) {
> +g_free(sops);
> return -TARGET_EFAULT;
> +}
>
> ret = -TARGET_ENOSYS;
> #ifdef __NR_semtimedop
> @@ -3928,6 +3936,7 @@ static inline abi_long do_semtimedop(int semid,
> SEMTIMEDOP_IPC_ARGS(nsops, sops,
> (long)pts)));
> }
> #endif
> +g_free(sops);
> return ret;
> }
> #endif
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index 3c261cff0e..f7f77346be 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -46,6 +46,8 @@
> #define IPCOP_shmget 23
> #define IPCOP_shmctl 24
>
> +#define TARGET_SEMOPM 500
> +
> /*
> * The following is for compatibility across the various Linux
> * platforms. The i386 ioctl numbering scheme doesn't really enforce
>
Reviewed-by: Laurent Vivier
