[Qemu-devel] [PATCH v3 01/13] linux-user/syscall: Verify recvfrom(addr) is user-writable

Philippe Mathieu-Daudé posted 13 patches 7 years, 4 months ago
Only 9 patches received!
There is a newer version of this series
[Qemu-devel] [PATCH v3 01/13] linux-user/syscall: Verify recvfrom(addr) is user-writable
Posted by Philippe Mathieu-Daudé 7 years, 4 months ago
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-By: Guido Günther <agx@sigxcpu.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 linux-user/syscall.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 2117fb13b4..ad40682cee 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -4154,6 +4154,11 @@ static abi_long do_recvfrom(int fd, abi_ulong msg, size_t len, int flags,
             ret = -TARGET_EINVAL;
             goto fail;
         }
+        if (!access_ok(VERIFY_WRITE, target_addr, addrlen)) {
+            ret = -TARGET_EFAULT;
+            goto fail;
+        }
+
         addr = alloca(addrlen);
         ret = get_errno(safe_recvfrom(fd, host_msg, len, flags,
                                       addr, &addrlen));
-- 
2.18.0


Re: [Qemu-devel] [PATCH v3 01/13] linux-user/syscall: Verify recvfrom(addr) is user-writable
Posted by Laurent Vivier 7 years, 4 months ago
Le 02/07/2018 à 19:50, Philippe Mathieu-Daudé a écrit :
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Tested-By: Guido Günther <agx@sigxcpu.org>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> ---
>  linux-user/syscall.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 2117fb13b4..ad40682cee 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -4154,6 +4154,11 @@ static abi_long do_recvfrom(int fd, abi_ulong msg, size_t len, int flags,
>              ret = -TARGET_EINVAL;
>              goto fail;
>          }
> +        if (!access_ok(VERIFY_WRITE, target_addr, addrlen)) {
> +            ret = -TARGET_EFAULT;
> +            goto fail;
> +        }
> +
>          addr = alloca(addrlen);
>          ret = get_errno(safe_recvfrom(fd, host_msg, len, flags,
>                                        addr, &addrlen));
> 

This patch breaks the test recvfrom01 of the Linux Test Project.

Laurent

Re: [Qemu-devel] [PATCH v3 01/13] linux-user/syscall: Verify recvfrom(addr) is user-writable
Posted by Philippe Mathieu-Daudé 7 years, 4 months ago
On 07/03/2018 11:55 AM, Laurent Vivier wrote:
> Le 02/07/2018 à 19:50, Philippe Mathieu-Daudé a écrit :
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> Tested-By: Guido Günther <agx@sigxcpu.org>
>> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
>> ---
>>  linux-user/syscall.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index 2117fb13b4..ad40682cee 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -4154,6 +4154,11 @@ static abi_long do_recvfrom(int fd, abi_ulong msg, size_t len, int flags,
>>              ret = -TARGET_EINVAL;
>>              goto fail;
>>          }
>> +        if (!access_ok(VERIFY_WRITE, target_addr, addrlen)) {
>> +            ret = -TARGET_EFAULT;
>> +            goto fail;
>> +        }
>> +
>>          addr = alloca(addrlen);
>>          ret = get_errno(safe_recvfrom(fd, host_msg, len, flags,
>>                                        addr, &addrlen));
>>
> 
> This patch breaks the test recvfrom01 of the Linux Test Project.

OK :(

I just sent v4, can you simply drop this patch?
I'll now look at it after 3.0.

Thanks,

Phil.

Re: [Qemu-devel] [PATCH v3 01/13] linux-user/syscall: Verify recvfrom(addr) is user-writable
Posted by Laurent Vivier 7 years, 4 months ago
Le 03/07/2018 à 17:39, Philippe Mathieu-Daudé a écrit :
> On 07/03/2018 11:55 AM, Laurent Vivier wrote:
>> Le 02/07/2018 à 19:50, Philippe Mathieu-Daudé a écrit :
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> Tested-By: Guido Günther <agx@sigxcpu.org>
>>> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
>>> ---
>>>  linux-user/syscall.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>>> index 2117fb13b4..ad40682cee 100644
>>> --- a/linux-user/syscall.c
>>> +++ b/linux-user/syscall.c
>>> @@ -4154,6 +4154,11 @@ static abi_long do_recvfrom(int fd, abi_ulong msg, size_t len, int flags,
>>>              ret = -TARGET_EINVAL;
>>>              goto fail;
>>>          }
>>> +        if (!access_ok(VERIFY_WRITE, target_addr, addrlen)) {
>>> +            ret = -TARGET_EFAULT;
>>> +            goto fail;
>>> +        }
>>> +
>>>          addr = alloca(addrlen);
>>>          ret = get_errno(safe_recvfrom(fd, host_msg, len, flags,
>>>                                        addr, &addrlen));
>>>
>>
>> This patch breaks the test recvfrom01 of the Linux Test Project.
> 
> OK :(
> 
> I just sent v4, can you simply drop this patch?
> I'll now look at it after 3.0.

Not enough time to add more patches to my pull request, sorry.

I'll queue them in my branch for the next release.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v3 01/13] linux-user/syscall: Verify recvfrom(addr) is user-writable
Posted by Philippe Mathieu-Daudé 7 years, 4 months ago
On 07/03/2018 01:38 PM, Laurent Vivier wrote:
> Le 03/07/2018 à 17:39, Philippe Mathieu-Daudé a écrit :
>> On 07/03/2018 11:55 AM, Laurent Vivier wrote:
>>> Le 02/07/2018 à 19:50, Philippe Mathieu-Daudé a écrit :
>>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>> Tested-By: Guido Günther <agx@sigxcpu.org>
>>>> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
>>>> ---
>>>>  linux-user/syscall.c | 5 +++++
>>>>  1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>>>> index 2117fb13b4..ad40682cee 100644
>>>> --- a/linux-user/syscall.c
>>>> +++ b/linux-user/syscall.c
>>>> @@ -4154,6 +4154,11 @@ static abi_long do_recvfrom(int fd, abi_ulong msg, size_t len, int flags,
>>>>              ret = -TARGET_EINVAL;
>>>>              goto fail;
>>>>          }
>>>> +        if (!access_ok(VERIFY_WRITE, target_addr, addrlen)) {
>>>> +            ret = -TARGET_EFAULT;
>>>> +            goto fail;
>>>> +        }
>>>> +
>>>>          addr = alloca(addrlen);
>>>>          ret = get_errno(safe_recvfrom(fd, host_msg, len, flags,
>>>>                                        addr, &addrlen));
>>>>
>>>
>>> This patch breaks the test recvfrom01 of the Linux Test Project.
>>
>> OK :(
>>
>> I just sent v4, can you simply drop this patch?
>> I'll now look at it after 3.0.
> 
> Not enough time to add more patches to my pull request, sorry.
> 
> I'll queue them in my branch for the next release.

Fair enough :)

Thanks!

Phil.