[PATCH] linux-user: allow NULL msg in recvfrom

Zach Reizner posted 1 patch 3 years, 1 month ago
Failed in applying to current master (apply log)
There is a newer version of this series
linux-user/syscall.c | 2 --
1 file changed, 2 deletions(-)
[PATCH] linux-user: allow NULL msg in recvfrom
Posted by Zach Reizner 3 years, 1 month ago
The kernel allows a NULL msg in recvfrom so that he size of the next
message may be queried before allocating a correctly sized buffer. This
change allows the syscall translator to pass along the NULL msg pointer
instead of returning early with EFAULT.

Signed-off-by: Zach Reizner <zachr@google.com>
---
 linux-user/syscall.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 1e508576c7..332544b43c 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -3680,8 +3680,6 @@ static abi_long do_recvfrom(int fd, abi_ulong
msg, size_t len, int flags,
     abi_long ret;

     host_msg = lock_user(VERIFY_WRITE, msg, len, 0);
-    if (!host_msg)
-        return -TARGET_EFAULT;
     if (target_addr) {
         if (get_user_u32(addrlen, target_addrlen)) {
             ret = -TARGET_EFAULT;
-- 
2.31.0.291.g576ba9dcdaf-goog

Re: [PATCH] linux-user: allow NULL msg in recvfrom
Posted by Laurent Vivier 3 years, 1 month ago
Le 26/03/2021 à 05:05, Zach Reizner a écrit :
> The kernel allows a NULL msg in recvfrom so that he size of the next
> message may be queried before allocating a correctly sized buffer. This
> change allows the syscall translator to pass along the NULL msg pointer
> instead of returning early with EFAULT.
> 
> Signed-off-by: Zach Reizner <zachr@google.com>
> ---
>  linux-user/syscall.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 1e508576c7..332544b43c 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -3680,8 +3680,6 @@ static abi_long do_recvfrom(int fd, abi_ulong
> msg, size_t len, int flags,
>      abi_long ret;
> 
>      host_msg = lock_user(VERIFY_WRITE, msg, len, 0);
> -    if (!host_msg)
> -        return -TARGET_EFAULT;
>      if (target_addr) {
>          if (get_user_u32(addrlen, target_addrlen)) {
>              ret = -TARGET_EFAULT;
> 

Applied to my linux-user-for-6.0 branch

Thanks,
Laurent

Re: [PATCH] linux-user: allow NULL msg in recvfrom
Posted by Peter Maydell 3 years, 1 month ago
On Fri, 26 Mar 2021 at 13:24, Laurent Vivier <laurent@vivier.eu> wrote:
>
> Le 26/03/2021 à 05:05, Zach Reizner a écrit :
> > The kernel allows a NULL msg in recvfrom so that he size of the next
> > message may be queried before allocating a correctly sized buffer. This
> > change allows the syscall translator to pass along the NULL msg pointer
> > instead of returning early with EFAULT.
> >
> > Signed-off-by: Zach Reizner <zachr@google.com>
> > ---
> >  linux-user/syscall.c | 2 --
> >  1 file changed, 2 deletions(-)
> >
> > diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> > index 1e508576c7..332544b43c 100644
> > --- a/linux-user/syscall.c
> > +++ b/linux-user/syscall.c
> > @@ -3680,8 +3680,6 @@ static abi_long do_recvfrom(int fd, abi_ulong
> > msg, size_t len, int flags,
> >      abi_long ret;
> >
> >      host_msg = lock_user(VERIFY_WRITE, msg, len, 0);
> > -    if (!host_msg)
> > -        return -TARGET_EFAULT;
> >      if (target_addr) {
> >          if (get_user_u32(addrlen, target_addrlen)) {
> >              ret = -TARGET_EFAULT;
> >
>
> Applied to my linux-user-for-6.0 branch

Doesn't this mean we'll now incorrectly treat "guest passed
a bad address" the same as "guest passed NULL" ? lock_user()
returns NULL for errors, so if you need to handle NULL input
specially you want something like

   if (!msg) {
       host_msg = NULL;
   } else {
       host_msg = lock_user(VERIFY_WRITE, msg, len, 0);
       if (!host_msg) {
           return -TARGET_EFAULT;
       }
   }

I think ?

thanks
-- PMM

Re: [PATCH] linux-user: allow NULL msg in recvfrom
Posted by Laurent Vivier 3 years, 1 month ago
Le 26/03/2021 à 14:28, Peter Maydell a écrit :
> On Fri, 26 Mar 2021 at 13:24, Laurent Vivier <laurent@vivier.eu> wrote:
>>
>> Le 26/03/2021 à 05:05, Zach Reizner a écrit :
>>> The kernel allows a NULL msg in recvfrom so that he size of the next
>>> message may be queried before allocating a correctly sized buffer. This
>>> change allows the syscall translator to pass along the NULL msg pointer
>>> instead of returning early with EFAULT.
>>>
>>> Signed-off-by: Zach Reizner <zachr@google.com>
>>> ---
>>>  linux-user/syscall.c | 2 --
>>>  1 file changed, 2 deletions(-)
>>>
>>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>>> index 1e508576c7..332544b43c 100644
>>> --- a/linux-user/syscall.c
>>> +++ b/linux-user/syscall.c
>>> @@ -3680,8 +3680,6 @@ static abi_long do_recvfrom(int fd, abi_ulong
>>> msg, size_t len, int flags,
>>>      abi_long ret;
>>>
>>>      host_msg = lock_user(VERIFY_WRITE, msg, len, 0);
>>> -    if (!host_msg)
>>> -        return -TARGET_EFAULT;
>>>      if (target_addr) {
>>>          if (get_user_u32(addrlen, target_addrlen)) {
>>>              ret = -TARGET_EFAULT;
>>>
>>
>> Applied to my linux-user-for-6.0 branch
> 
> Doesn't this mean we'll now incorrectly treat "guest passed
> a bad address" the same as "guest passed NULL" ? lock_user()
> returns NULL for errors, so if you need to handle NULL input
> specially you want something like
> 
>    if (!msg) {
>        host_msg = NULL;
>    } else {
>        host_msg = lock_user(VERIFY_WRITE, msg, len, 0);
>        if (!host_msg) {
>            return -TARGET_EFAULT;
>        }
>    }
> 
> I think ?

Yes, you're right.

Zach, could you update your patch?

Thanks,
Laurent


Re: [PATCH] linux-user: allow NULL msg in recvfrom
Posted by Laurent Vivier 3 years, 1 month ago
Le 26/03/2021 à 05:05, Zach Reizner a écrit :
> The kernel allows a NULL msg in recvfrom so that he size of the next
> message may be queried before allocating a correctly sized buffer. This
> change allows the syscall translator to pass along the NULL msg pointer
> instead of returning early with EFAULT.
> 
> Signed-off-by: Zach Reizner <zachr@google.com>
> ---
>  linux-user/syscall.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 1e508576c7..332544b43c 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -3680,8 +3680,6 @@ static abi_long do_recvfrom(int fd, abi_ulong
> msg, size_t len, int flags,
>      abi_long ret;
> 
>      host_msg = lock_user(VERIFY_WRITE, msg, len, 0);
> -    if (!host_msg)
> -        return -TARGET_EFAULT;
>      if (target_addr) {
>          if (get_user_u32(addrlen, target_addrlen)) {
>              ret = -TARGET_EFAULT;
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>