* [PATCH] linux-user: Check lock_user result for ip_mreq_source sockopts
@ 2021-08-09 15:54 Peter Maydell
2021-08-09 16:00 ` Philippe Mathieu-Daudé
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Peter Maydell @ 2021-08-09 15:54 UTC (permalink / raw)
To: qemu-devel; +Cc: Laurent Vivier
In do_setsockopt(), the code path for the options which take a struct
ip_mreq_source (IP_BLOCK_SOURCE, IP_UNBLOCK_SOURCE,
IP_ADD_SOURCE_MEMBERSHIP and IP_DROP_SOURCE_MEMBERSHIP) fails to
check the return value from lock_user(). Handle this in the usual
way by returning -TARGET_EFAULT.
(In practice this was probably harmless because we'd pass a NULL
pointer to setsockopt() and the kernel would then return EFAULT.)
Fixes: Coverity CID 1459987
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Compile-tested only; I don't have a test case to hand that
uses these socket options.
linux-user/syscall.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index ccd3892b2df..d2b062ea5a9 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -2121,6 +2121,9 @@ static abi_long do_setsockopt(int sockfd, int level, int optname,
return -TARGET_EINVAL;
ip_mreq_source = lock_user(VERIFY_READ, optval_addr, optlen, 1);
+ if (!ip_mreq_source) {
+ return -TARGET_EFAULT;
+ }
ret = get_errno(setsockopt(sockfd, level, optname, ip_mreq_source, optlen));
unlock_user (ip_mreq_source, optval_addr, 0);
break;
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] linux-user: Check lock_user result for ip_mreq_source sockopts
2021-08-09 15:54 [PATCH] linux-user: Check lock_user result for ip_mreq_source sockopts Peter Maydell
@ 2021-08-09 16:00 ` Philippe Mathieu-Daudé
2021-08-10 13:25 ` Laurent Vivier
2021-09-13 19:20 ` Laurent Vivier
2 siblings, 0 replies; 4+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-08-09 16:00 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: Laurent Vivier
On 8/9/21 5:54 PM, Peter Maydell wrote:
> In do_setsockopt(), the code path for the options which take a struct
> ip_mreq_source (IP_BLOCK_SOURCE, IP_UNBLOCK_SOURCE,
> IP_ADD_SOURCE_MEMBERSHIP and IP_DROP_SOURCE_MEMBERSHIP) fails to
> check the return value from lock_user(). Handle this in the usual
> way by returning -TARGET_EFAULT.
>
> (In practice this was probably harmless because we'd pass a NULL
> pointer to setsockopt() and the kernel would then return EFAULT.)
>
> Fixes: Coverity CID 1459987
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Compile-tested only; I don't have a test case to hand that
> uses these socket options.
>
> linux-user/syscall.c | 3 +++
> 1 file changed, 3 insertions(+)
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] linux-user: Check lock_user result for ip_mreq_source sockopts
2021-08-09 15:54 [PATCH] linux-user: Check lock_user result for ip_mreq_source sockopts Peter Maydell
2021-08-09 16:00 ` Philippe Mathieu-Daudé
@ 2021-08-10 13:25 ` Laurent Vivier
2021-09-13 19:20 ` Laurent Vivier
2 siblings, 0 replies; 4+ messages in thread
From: Laurent Vivier @ 2021-08-10 13:25 UTC (permalink / raw)
To: Peter Maydell, qemu-devel
Le 09/08/2021 à 17:54, Peter Maydell a écrit :
> In do_setsockopt(), the code path for the options which take a struct
> ip_mreq_source (IP_BLOCK_SOURCE, IP_UNBLOCK_SOURCE,
> IP_ADD_SOURCE_MEMBERSHIP and IP_DROP_SOURCE_MEMBERSHIP) fails to
> check the return value from lock_user(). Handle this in the usual
> way by returning -TARGET_EFAULT.
>
> (In practice this was probably harmless because we'd pass a NULL
> pointer to setsockopt() and the kernel would then return EFAULT.)
>
> Fixes: Coverity CID 1459987
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Compile-tested only; I don't have a test case to hand that
> uses these socket options.
>
> linux-user/syscall.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index ccd3892b2df..d2b062ea5a9 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -2121,6 +2121,9 @@ static abi_long do_setsockopt(int sockfd, int level, int optname,
> return -TARGET_EINVAL;
>
> ip_mreq_source = lock_user(VERIFY_READ, optval_addr, optlen, 1);
> + if (!ip_mreq_source) {
> + return -TARGET_EFAULT;
> + }
> ret = get_errno(setsockopt(sockfd, level, optname, ip_mreq_source, optlen));
> unlock_user (ip_mreq_source, optval_addr, 0);
> break;
>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] linux-user: Check lock_user result for ip_mreq_source sockopts
2021-08-09 15:54 [PATCH] linux-user: Check lock_user result for ip_mreq_source sockopts Peter Maydell
2021-08-09 16:00 ` Philippe Mathieu-Daudé
2021-08-10 13:25 ` Laurent Vivier
@ 2021-09-13 19:20 ` Laurent Vivier
2 siblings, 0 replies; 4+ messages in thread
From: Laurent Vivier @ 2021-09-13 19:20 UTC (permalink / raw)
To: Peter Maydell, qemu-devel
Le 09/08/2021 à 17:54, Peter Maydell a écrit :
> In do_setsockopt(), the code path for the options which take a struct
> ip_mreq_source (IP_BLOCK_SOURCE, IP_UNBLOCK_SOURCE,
> IP_ADD_SOURCE_MEMBERSHIP and IP_DROP_SOURCE_MEMBERSHIP) fails to
> check the return value from lock_user(). Handle this in the usual
> way by returning -TARGET_EFAULT.
>
> (In practice this was probably harmless because we'd pass a NULL
> pointer to setsockopt() and the kernel would then return EFAULT.)
>
> Fixes: Coverity CID 1459987
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Compile-tested only; I don't have a test case to hand that
> uses these socket options.
>
> linux-user/syscall.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index ccd3892b2df..d2b062ea5a9 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -2121,6 +2121,9 @@ static abi_long do_setsockopt(int sockfd, int level, int optname,
> return -TARGET_EINVAL;
>
> ip_mreq_source = lock_user(VERIFY_READ, optval_addr, optlen, 1);
> + if (!ip_mreq_source) {
> + return -TARGET_EFAULT;
> + }
> ret = get_errno(setsockopt(sockfd, level, optname, ip_mreq_source, optlen));
> unlock_user (ip_mreq_source, optval_addr, 0);
> break;
>
Applied to my linux-user-for-6.2 branch.
Thanks,
Laurent
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-09-13 19:21 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-09 15:54 [PATCH] linux-user: Check lock_user result for ip_mreq_source sockopts Peter Maydell
2021-08-09 16:00 ` Philippe Mathieu-Daudé
2021-08-10 13:25 ` Laurent Vivier
2021-09-13 19:20 ` Laurent Vivier
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.