All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] linux-user: fix emulation of accept4/getpeername/getsockname/recvfrom syscalls
@ 2019-02-07 14:51 Andreas Schwab
  2019-02-07 15:22 ` no-reply
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Andreas Schwab @ 2019-02-07 14:51 UTC (permalink / raw)
  To: qemu-devel; +Cc: Riku Voipio, Laurent Vivier

System calls that return a socket address do so by writing the (possibly
truncated) address into the provided buffer space, but setting the addrlen
parameter to the actual size of the address.  To determine how much to
copy back to the target memory the emulation needs to remember the old
value of the addrlen parameter, so that it doesn't write past the buffer
limits.

Signed-off-by: Andreas Schwab <schwab@suse.de>
---
 linux-user/syscall.c | 38 ++++++++++++++++++++++----------------
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 55fa235a56..90bfda3563 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -2884,7 +2884,7 @@ static abi_long do_sendrecvmmsg(int fd, abi_ulong target_msgvec,
 static abi_long do_accept4(int fd, abi_ulong target_addr,
                            abi_ulong target_addrlen_addr, int flags)
 {
-    socklen_t addrlen;
+    socklen_t addrlen, ret_addrlen;
     void *addr;
     abi_long ret;
     int host_flags;
@@ -2908,10 +2908,11 @@ static abi_long do_accept4(int fd, abi_ulong target_addr,
 
     addr = alloca(addrlen);
 
-    ret = get_errno(safe_accept4(fd, addr, &addrlen, host_flags));
+    ret_addrlen = addrlen;
+    ret = get_errno(safe_accept4(fd, addr, &ret_addrlen, host_flags));
     if (!is_error(ret)) {
-        host_to_target_sockaddr(target_addr, addr, addrlen);
-        if (put_user_u32(addrlen, target_addrlen_addr))
+        host_to_target_sockaddr(target_addr, addr, MIN(addrlen, ret_addrlen));
+        if (put_user_u32(ret_addrlen, target_addrlen_addr))
             ret = -TARGET_EFAULT;
     }
     return ret;
@@ -2921,7 +2922,7 @@ static abi_long do_accept4(int fd, abi_ulong target_addr,
 static abi_long do_getpeername(int fd, abi_ulong target_addr,
                                abi_ulong target_addrlen_addr)
 {
-    socklen_t addrlen;
+    socklen_t addrlen, ret_addrlen;
     void *addr;
     abi_long ret;
 
@@ -2937,10 +2938,11 @@ static abi_long do_getpeername(int fd, abi_ulong target_addr,
 
     addr = alloca(addrlen);
 
-    ret = get_errno(getpeername(fd, addr, &addrlen));
+    ret_addrlen = addrlen;
+    ret = get_errno(getpeername(fd, addr, &ret_addrlen));
     if (!is_error(ret)) {
-        host_to_target_sockaddr(target_addr, addr, addrlen);
-        if (put_user_u32(addrlen, target_addrlen_addr))
+        host_to_target_sockaddr(target_addr, addr, MIN(addrlen, ret_addrlen));
+        if (put_user_u32(ret_addrlen, target_addrlen_addr))
             ret = -TARGET_EFAULT;
     }
     return ret;
@@ -2950,7 +2952,7 @@ static abi_long do_getpeername(int fd, abi_ulong target_addr,
 static abi_long do_getsockname(int fd, abi_ulong target_addr,
                                abi_ulong target_addrlen_addr)
 {
-    socklen_t addrlen;
+    socklen_t addrlen, ret_addrlen;
     void *addr;
     abi_long ret;
 
@@ -2966,10 +2968,11 @@ static abi_long do_getsockname(int fd, abi_ulong target_addr,
 
     addr = alloca(addrlen);
 
-    ret = get_errno(getsockname(fd, addr, &addrlen));
+    ret_addrlen = addrlen;
+    ret = get_errno(getsockname(fd, addr, &ret_addrlen));
     if (!is_error(ret)) {
-        host_to_target_sockaddr(target_addr, addr, addrlen);
-        if (put_user_u32(addrlen, target_addrlen_addr))
+        host_to_target_sockaddr(target_addr, addr, MIN(addrlen, ret_addrlen));
+        if (put_user_u32(ret_addrlen, target_addrlen_addr))
             ret = -TARGET_EFAULT;
     }
     return ret;
@@ -3042,7 +3045,7 @@ static abi_long do_recvfrom(int fd, abi_ulong msg, size_t len, int flags,
                             abi_ulong target_addr,
                             abi_ulong target_addrlen)
 {
-    socklen_t addrlen;
+    socklen_t addrlen, ret_addrlen;
     void *addr;
     void *host_msg;
     abi_long ret;
@@ -3060,10 +3063,12 @@ static abi_long do_recvfrom(int fd, abi_ulong msg, size_t len, int flags,
             goto fail;
         }
         addr = alloca(addrlen);
+        ret_addrlen = addrlen;
         ret = get_errno(safe_recvfrom(fd, host_msg, len, flags,
-                                      addr, &addrlen));
+                                      addr, &ret_addrlen));
     } else {
         addr = NULL; /* To keep compiler quiet.  */
+        addrlen = 0; /* To keep compiler quiet.  */
         ret = get_errno(safe_recvfrom(fd, host_msg, len, flags, NULL, 0));
     }
     if (!is_error(ret)) {
@@ -3076,8 +3081,9 @@ static abi_long do_recvfrom(int fd, abi_ulong msg, size_t len, int flags,
             }
         }
         if (target_addr) {
-            host_to_target_sockaddr(target_addr, addr, addrlen);
-            if (put_user_u32(addrlen, target_addrlen)) {
+            host_to_target_sockaddr(target_addr, addr,
+                                    MIN(addrlen, ret_addrlen));
+            if (put_user_u32(ret_addrlen, target_addrlen)) {
                 ret = -TARGET_EFAULT;
                 goto fail;
             }
-- 
2.20.1


-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH] linux-user: fix emulation of accept4/getpeername/getsockname/recvfrom syscalls
  2019-02-07 14:51 [Qemu-devel] [PATCH] linux-user: fix emulation of accept4/getpeername/getsockname/recvfrom syscalls Andreas Schwab
@ 2019-02-07 15:22 ` no-reply
  2019-02-07 15:26 ` no-reply
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: no-reply @ 2019-02-07 15:22 UTC (permalink / raw)
  To: schwab; +Cc: fam, qemu-devel, riku.voipio, laurent

Patchew URL: https://patchew.org/QEMU/mvmmun7y7m0.fsf@suse.de/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PATCH] linux-user: fix emulation of accept4/getpeername/getsockname/recvfrom syscalls
Type: series
Message-id: mvmmun7y7m0.fsf@suse.de

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
   1dca054955..632351e0e1  master     -> master
 t [tag update]            patchew/20190207102445.71998-1-vsementsov@virtuozzo.com -> patchew/20190207102445.71998-1-vsementsov@virtuozzo.com
 * [new tag]               patchew/mvmmun7y7m0.fsf@suse.de -> patchew/mvmmun7y7m0.fsf@suse.de
Switched to a new branch 'test'
276d8b2c5d linux-user: fix emulation of accept4/getpeername/getsockname/recvfrom syscalls

=== OUTPUT BEGIN ===
ERROR: braces {} are necessary for all arms of this statement
#41: FILE: linux-user/syscall.c:2877:
+        if (put_user_u32(ret_addrlen, target_addrlen_addr))
[...]

ERROR: braces {} are necessary for all arms of this statement
#65: FILE: linux-user/syscall.c:2907:
+        if (put_user_u32(ret_addrlen, target_addrlen_addr))
[...]

ERROR: braces {} are necessary for all arms of this statement
#89: FILE: linux-user/syscall.c:2937:
+        if (put_user_u32(ret_addrlen, target_addrlen_addr))
[...]

total: 3 errors, 0 warnings, 98 lines checked

Commit 276d8b2c5d7b (linux-user: fix emulation of accept4/getpeername/getsockname/recvfrom syscalls) has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/mvmmun7y7m0.fsf@suse.de/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH] linux-user: fix emulation of accept4/getpeername/getsockname/recvfrom syscalls
  2019-02-07 14:51 [Qemu-devel] [PATCH] linux-user: fix emulation of accept4/getpeername/getsockname/recvfrom syscalls Andreas Schwab
  2019-02-07 15:22 ` no-reply
@ 2019-02-07 15:26 ` no-reply
  2019-02-14  9:17 ` Laurent Vivier
  2019-02-14 13:25 ` Laurent Vivier
  3 siblings, 0 replies; 9+ messages in thread
From: no-reply @ 2019-02-07 15:26 UTC (permalink / raw)
  To: schwab; +Cc: fam, qemu-devel, riku.voipio, laurent

Patchew URL: https://patchew.org/QEMU/mvmmun7y7m0.fsf@suse.de/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PATCH] linux-user: fix emulation of accept4/getpeername/getsockname/recvfrom syscalls
Message-id: mvmmun7y7m0.fsf@suse.de
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
   1dca054..632351e  master     -> master
 - [tag update]      patchew/20190207102445.71998-1-vsementsov@virtuozzo.com -> patchew/20190207102445.71998-1-vsementsov@virtuozzo.com
 * [new tag]         patchew/mvmmun7y7m0.fsf@suse.de -> patchew/mvmmun7y7m0.fsf@suse.de
Submodule 'capstone' (https://git.qemu.org/git/capstone.git) registered for path 'capstone'
Submodule 'dtc' (https://git.qemu.org/git/dtc.git) registered for path 'dtc'
Submodule 'roms/QemuMacDrivers' (https://git.qemu.org/git/QemuMacDrivers.git) registered for path 'roms/QemuMacDrivers'
Submodule 'roms/SLOF' (https://git.qemu.org/git/SLOF.git) registered for path 'roms/SLOF'
Submodule 'roms/ipxe' (https://git.qemu.org/git/ipxe.git) registered for path 'roms/ipxe'
Submodule 'roms/openbios' (https://git.qemu.org/git/openbios.git) registered for path 'roms/openbios'
Submodule 'roms/openhackware' (https://git.qemu.org/git/openhackware.git) registered for path 'roms/openhackware'
Submodule 'roms/qemu-palcode' (https://git.qemu.org/git/qemu-palcode.git) registered for path 'roms/qemu-palcode'
Submodule 'roms/seabios' (https://git.qemu.org/git/seabios.git/) registered for path 'roms/seabios'
Submodule 'roms/seabios-hppa' (https://github.com/hdeller/seabios-hppa.git) registered for path 'roms/seabios-hppa'
Submodule 'roms/sgabios' (https://git.qemu.org/git/sgabios.git) registered for path 'roms/sgabios'
Submodule 'roms/skiboot' (https://git.qemu.org/git/skiboot.git) registered for path 'roms/skiboot'
Submodule 'roms/u-boot' (https://git.qemu.org/git/u-boot.git) registered for path 'roms/u-boot'
Submodule 'roms/u-boot-sam460ex' (https://git.qemu.org/git/u-boot-sam460ex.git) registered for path 'roms/u-boot-sam460ex'
Submodule 'tests/fp/berkeley-softfloat-3' (https://github.com/cota/berkeley-softfloat-3) registered for path 'tests/fp/berkeley-softfloat-3'
Submodule 'tests/fp/berkeley-testfloat-3' (https://github.com/cota/berkeley-testfloat-3) registered for path 'tests/fp/berkeley-testfloat-3'
Submodule 'ui/keycodemapdb' (https://git.qemu.org/git/keycodemapdb.git) registered for path 'ui/keycodemapdb'
Cloning into 'capstone'...
Submodule path 'capstone': checked out '22ead3e0bfdb87516656453336160e0a37b066bf'
Cloning into 'dtc'...
Submodule path 'dtc': checked out '88f18909db731a627456f26d779445f84e449536'
Cloning into 'roms/QemuMacDrivers'...
Submodule path 'roms/QemuMacDrivers': checked out '90c488d5f4a407342247b9ea869df1c2d9c8e266'
Cloning into 'roms/SLOF'...
Submodule path 'roms/SLOF': checked out 'a5b428e1c1eae703bdd62a3f527223c291ee3fdc'
Cloning into 'roms/ipxe'...
Submodule path 'roms/ipxe': checked out 'de4565cbe76ea9f7913a01f331be3ee901bb6e17'
Cloning into 'roms/openbios'...
Submodule path 'roms/openbios': checked out '441a84d3a642a10b948369c63f32367e8ff6395b'
Cloning into 'roms/openhackware'...
Submodule path 'roms/openhackware': checked out 'c559da7c8eec5e45ef1f67978827af6f0b9546f5'
Cloning into 'roms/qemu-palcode'...
Submodule path 'roms/qemu-palcode': checked out '51c237d7e20d05100eacadee2f61abc17e6bc097'
Cloning into 'roms/seabios'...
Submodule path 'roms/seabios': checked out 'a698c8995ffb2838296ec284fe3c4ad33dfca307'
Cloning into 'roms/seabios-hppa'...
Submodule path 'roms/seabios-hppa': checked out '1ef99a01572c2581c30e16e6fe69e9ea2ef92ce0'
Cloning into 'roms/sgabios'...
Submodule path 'roms/sgabios': checked out 'cbaee52287e5f32373181cff50a00b6c4ac9015a'
Cloning into 'roms/skiboot'...
Submodule path 'roms/skiboot': checked out 'e0ee24c27a172bcf482f6f2bc905e6211c134bcc'
Cloning into 'roms/u-boot'...
Submodule path 'roms/u-boot': checked out 'd85ca029f257b53a96da6c2fb421e78a003a9943'
Cloning into 'roms/u-boot-sam460ex'...
Submodule path 'roms/u-boot-sam460ex': checked out '60b3916f33e617a815973c5a6df77055b2e3a588'
Cloning into 'tests/fp/berkeley-softfloat-3'...
Submodule path 'tests/fp/berkeley-softfloat-3': checked out 'b64af41c3276f97f0e181920400ee056b9c88037'
Cloning into 'tests/fp/berkeley-testfloat-3'...
Submodule path 'tests/fp/berkeley-testfloat-3': checked out '5a59dcec19327396a011a17fd924aed4fec416b3'
Cloning into 'ui/keycodemapdb'...
Submodule path 'ui/keycodemapdb': checked out '6b3d716e2b6472eb7189d3220552280ef3d832ce'
Switched to a new branch 'test'
276d8b2 linux-user: fix emulation of accept4/getpeername/getsockname/recvfrom syscalls

=== OUTPUT BEGIN ===
ERROR: braces {} are necessary for all arms of this statement
#41: FILE: linux-user/syscall.c:2877:
+        if (put_user_u32(ret_addrlen, target_addrlen_addr))
[...]

ERROR: braces {} are necessary for all arms of this statement
#65: FILE: linux-user/syscall.c:2907:
+        if (put_user_u32(ret_addrlen, target_addrlen_addr))
[...]

ERROR: braces {} are necessary for all arms of this statement
#89: FILE: linux-user/syscall.c:2937:
+        if (put_user_u32(ret_addrlen, target_addrlen_addr))
[...]

total: 3 errors, 0 warnings, 98 lines checked

Commit 276d8b2c5d7b (linux-user: fix emulation of accept4/getpeername/getsockname/recvfrom syscalls) has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/mvmmun7y7m0.fsf@suse.de/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH] linux-user: fix emulation of accept4/getpeername/getsockname/recvfrom syscalls
  2019-02-07 14:51 [Qemu-devel] [PATCH] linux-user: fix emulation of accept4/getpeername/getsockname/recvfrom syscalls Andreas Schwab
  2019-02-07 15:22 ` no-reply
  2019-02-07 15:26 ` no-reply
@ 2019-02-14  9:17 ` Laurent Vivier
  2019-02-14 11:43   ` Andreas Schwab
  2019-02-14 13:25 ` Laurent Vivier
  3 siblings, 1 reply; 9+ messages in thread
From: Laurent Vivier @ 2019-02-14  9:17 UTC (permalink / raw)
  To: Andreas Schwab, qemu-devel; +Cc: Riku Voipio

On 07/02/2019 15:51, Andreas Schwab wrote:
> System calls that return a socket address do so by writing the (possibly
> truncated) address into the provided buffer space, but setting the addrlen
> parameter to the actual size of the address.  To determine how much to
> copy back to the target memory the emulation needs to remember the old
> value of the addrlen parameter, so that it doesn't write past the buffer
> limits.
> 
> Signed-off-by: Andreas Schwab <schwab@suse.de>

Andreas,

could you fix the coding style errors reported by patchew?

Thanks,
Laurent

> ---
>  linux-user/syscall.c | 38 ++++++++++++++++++++++----------------
>  1 file changed, 22 insertions(+), 16 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 55fa235a56..90bfda3563 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -2884,7 +2884,7 @@ static abi_long do_sendrecvmmsg(int fd, abi_ulong target_msgvec,
>  static abi_long do_accept4(int fd, abi_ulong target_addr,
>                             abi_ulong target_addrlen_addr, int flags)
>  {
> -    socklen_t addrlen;
> +    socklen_t addrlen, ret_addrlen;
>      void *addr;
>      abi_long ret;
>      int host_flags;
> @@ -2908,10 +2908,11 @@ static abi_long do_accept4(int fd, abi_ulong target_addr,
>  
>      addr = alloca(addrlen);
>  
> -    ret = get_errno(safe_accept4(fd, addr, &addrlen, host_flags));
> +    ret_addrlen = addrlen;
> +    ret = get_errno(safe_accept4(fd, addr, &ret_addrlen, host_flags));
>      if (!is_error(ret)) {
> -        host_to_target_sockaddr(target_addr, addr, addrlen);
> -        if (put_user_u32(addrlen, target_addrlen_addr))
> +        host_to_target_sockaddr(target_addr, addr, MIN(addrlen, ret_addrlen));
> +        if (put_user_u32(ret_addrlen, target_addrlen_addr))
>              ret = -TARGET_EFAULT;
>      }
>      return ret;
> @@ -2921,7 +2922,7 @@ static abi_long do_accept4(int fd, abi_ulong target_addr,
>  static abi_long do_getpeername(int fd, abi_ulong target_addr,
>                                 abi_ulong target_addrlen_addr)
>  {
> -    socklen_t addrlen;
> +    socklen_t addrlen, ret_addrlen;
>      void *addr;
>      abi_long ret;
>  
> @@ -2937,10 +2938,11 @@ static abi_long do_getpeername(int fd, abi_ulong target_addr,
>  
>      addr = alloca(addrlen);
>  
> -    ret = get_errno(getpeername(fd, addr, &addrlen));
> +    ret_addrlen = addrlen;
> +    ret = get_errno(getpeername(fd, addr, &ret_addrlen));
>      if (!is_error(ret)) {
> -        host_to_target_sockaddr(target_addr, addr, addrlen);
> -        if (put_user_u32(addrlen, target_addrlen_addr))
> +        host_to_target_sockaddr(target_addr, addr, MIN(addrlen, ret_addrlen));
> +        if (put_user_u32(ret_addrlen, target_addrlen_addr))
>              ret = -TARGET_EFAULT;
>      }
>      return ret;
> @@ -2950,7 +2952,7 @@ static abi_long do_getpeername(int fd, abi_ulong target_addr,
>  static abi_long do_getsockname(int fd, abi_ulong target_addr,
>                                 abi_ulong target_addrlen_addr)
>  {
> -    socklen_t addrlen;
> +    socklen_t addrlen, ret_addrlen;
>      void *addr;
>      abi_long ret;
>  
> @@ -2966,10 +2968,11 @@ static abi_long do_getsockname(int fd, abi_ulong target_addr,
>  
>      addr = alloca(addrlen);
>  
> -    ret = get_errno(getsockname(fd, addr, &addrlen));
> +    ret_addrlen = addrlen;
> +    ret = get_errno(getsockname(fd, addr, &ret_addrlen));
>      if (!is_error(ret)) {
> -        host_to_target_sockaddr(target_addr, addr, addrlen);
> -        if (put_user_u32(addrlen, target_addrlen_addr))
> +        host_to_target_sockaddr(target_addr, addr, MIN(addrlen, ret_addrlen));
> +        if (put_user_u32(ret_addrlen, target_addrlen_addr))
>              ret = -TARGET_EFAULT;
>      }
>      return ret;
> @@ -3042,7 +3045,7 @@ static abi_long do_recvfrom(int fd, abi_ulong msg, size_t len, int flags,
>                              abi_ulong target_addr,
>                              abi_ulong target_addrlen)
>  {
> -    socklen_t addrlen;
> +    socklen_t addrlen, ret_addrlen;
>      void *addr;
>      void *host_msg;
>      abi_long ret;
> @@ -3060,10 +3063,12 @@ static abi_long do_recvfrom(int fd, abi_ulong msg, size_t len, int flags,
>              goto fail;
>          }
>          addr = alloca(addrlen);
> +        ret_addrlen = addrlen;
>          ret = get_errno(safe_recvfrom(fd, host_msg, len, flags,
> -                                      addr, &addrlen));
> +                                      addr, &ret_addrlen));
>      } else {
>          addr = NULL; /* To keep compiler quiet.  */
> +        addrlen = 0; /* To keep compiler quiet.  */
>          ret = get_errno(safe_recvfrom(fd, host_msg, len, flags, NULL, 0));
>      }
>      if (!is_error(ret)) {
> @@ -3076,8 +3081,9 @@ static abi_long do_recvfrom(int fd, abi_ulong msg, size_t len, int flags,
>              }
>          }
>          if (target_addr) {
> -            host_to_target_sockaddr(target_addr, addr, addrlen);
> -            if (put_user_u32(addrlen, target_addrlen)) {
> +            host_to_target_sockaddr(target_addr, addr,
> +                                    MIN(addrlen, ret_addrlen));
> +            if (put_user_u32(ret_addrlen, target_addrlen)) {
>                  ret = -TARGET_EFAULT;
>                  goto fail;
>              }
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Qemu-devel] [PATCH] linux-user: fix emulation of accept4/getpeername/getsockname/recvfrom syscalls
  2019-02-14  9:17 ` Laurent Vivier
@ 2019-02-14 11:43   ` Andreas Schwab
  2019-02-14 13:27     ` Laurent Vivier
  0 siblings, 1 reply; 9+ messages in thread
From: Andreas Schwab @ 2019-02-14 11:43 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: qemu-devel, Riku Voipio

System calls that return a socket address do so by writing the (possibly
truncated) address into the provided buffer space, but setting the
addrlen parameter to the actual size of the address.  To determine how
much to copy back to the target memory the emulation needs to remember
the incoming value of the addrlen parameter, so that it doesn't write
past the buffer limits.

Signed-off-by: Andreas Schwab <schwab@suse.de>
---
 linux-user/syscall.c | 41 +++++++++++++++++++++++++----------------
 1 file changed, 25 insertions(+), 16 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 4a320850bb..f24743a2b2 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -2885,7 +2885,7 @@ static abi_long do_sendrecvmmsg(int fd, abi_ulong target_msgvec,
 static abi_long do_accept4(int fd, abi_ulong target_addr,
                            abi_ulong target_addrlen_addr, int flags)
 {
-    socklen_t addrlen;
+    socklen_t addrlen, ret_addrlen;
     void *addr;
     abi_long ret;
     int host_flags;
@@ -2909,11 +2909,13 @@ static abi_long do_accept4(int fd, abi_ulong target_addr,
 
     addr = alloca(addrlen);
 
-    ret = get_errno(safe_accept4(fd, addr, &addrlen, host_flags));
+    ret_addrlen = addrlen;
+    ret = get_errno(safe_accept4(fd, addr, &ret_addrlen, host_flags));
     if (!is_error(ret)) {
-        host_to_target_sockaddr(target_addr, addr, addrlen);
-        if (put_user_u32(addrlen, target_addrlen_addr))
+        host_to_target_sockaddr(target_addr, addr, MIN(addrlen, ret_addrlen));
+        if (put_user_u32(ret_addrlen, target_addrlen_addr)) {
             ret = -TARGET_EFAULT;
+        }
     }
     return ret;
 }
@@ -2922,7 +2924,7 @@ static abi_long do_accept4(int fd, abi_ulong target_addr,
 static abi_long do_getpeername(int fd, abi_ulong target_addr,
                                abi_ulong target_addrlen_addr)
 {
-    socklen_t addrlen;
+    socklen_t addrlen, ret_addrlen;
     void *addr;
     abi_long ret;
 
@@ -2938,11 +2940,13 @@ static abi_long do_getpeername(int fd, abi_ulong target_addr,
 
     addr = alloca(addrlen);
 
-    ret = get_errno(getpeername(fd, addr, &addrlen));
+    ret_addrlen = addrlen;
+    ret = get_errno(getpeername(fd, addr, &ret_addrlen));
     if (!is_error(ret)) {
-        host_to_target_sockaddr(target_addr, addr, addrlen);
-        if (put_user_u32(addrlen, target_addrlen_addr))
+        host_to_target_sockaddr(target_addr, addr, MIN(addrlen, ret_addrlen));
+        if (put_user_u32(ret_addrlen, target_addrlen_addr)) {
             ret = -TARGET_EFAULT;
+        }
     }
     return ret;
 }
@@ -2951,7 +2955,7 @@ static abi_long do_getpeername(int fd, abi_ulong target_addr,
 static abi_long do_getsockname(int fd, abi_ulong target_addr,
                                abi_ulong target_addrlen_addr)
 {
-    socklen_t addrlen;
+    socklen_t addrlen, ret_addrlen;
     void *addr;
     abi_long ret;
 
@@ -2967,11 +2971,13 @@ static abi_long do_getsockname(int fd, abi_ulong target_addr,
 
     addr = alloca(addrlen);
 
-    ret = get_errno(getsockname(fd, addr, &addrlen));
+    ret_addrlen = addrlen;
+    ret = get_errno(getsockname(fd, addr, &ret_addrlen));
     if (!is_error(ret)) {
-        host_to_target_sockaddr(target_addr, addr, addrlen);
-        if (put_user_u32(addrlen, target_addrlen_addr))
+        host_to_target_sockaddr(target_addr, addr, MIN(addrlen, ret_addrlen));
+        if (put_user_u32(ret_addrlen, target_addrlen_addr)) {
             ret = -TARGET_EFAULT;
+        }
     }
     return ret;
 }
@@ -3043,7 +3049,7 @@ static abi_long do_recvfrom(int fd, abi_ulong msg, size_t len, int flags,
                             abi_ulong target_addr,
                             abi_ulong target_addrlen)
 {
-    socklen_t addrlen;
+    socklen_t addrlen, ret_addrlen;
     void *addr;
     void *host_msg;
     abi_long ret;
@@ -3061,10 +3067,12 @@ static abi_long do_recvfrom(int fd, abi_ulong msg, size_t len, int flags,
             goto fail;
         }
         addr = alloca(addrlen);
+        ret_addrlen = addrlen;
         ret = get_errno(safe_recvfrom(fd, host_msg, len, flags,
-                                      addr, &addrlen));
+                                      addr, &ret_addrlen));
     } else {
         addr = NULL; /* To keep compiler quiet.  */
+        addrlen = 0; /* To keep compiler quiet.  */
         ret = get_errno(safe_recvfrom(fd, host_msg, len, flags, NULL, 0));
     }
     if (!is_error(ret)) {
@@ -3077,8 +3085,9 @@ static abi_long do_recvfrom(int fd, abi_ulong msg, size_t len, int flags,
             }
         }
         if (target_addr) {
-            host_to_target_sockaddr(target_addr, addr, addrlen);
-            if (put_user_u32(addrlen, target_addrlen)) {
+            host_to_target_sockaddr(target_addr, addr,
+                                    MIN(addrlen, ret_addrlen));
+            if (put_user_u32(ret_addrlen, target_addrlen)) {
                 ret = -TARGET_EFAULT;
                 goto fail;
             }
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH] linux-user: fix emulation of accept4/getpeername/getsockname/recvfrom syscalls
  2019-02-07 14:51 [Qemu-devel] [PATCH] linux-user: fix emulation of accept4/getpeername/getsockname/recvfrom syscalls Andreas Schwab
                   ` (2 preceding siblings ...)
  2019-02-14  9:17 ` Laurent Vivier
@ 2019-02-14 13:25 ` Laurent Vivier
  3 siblings, 0 replies; 9+ messages in thread
From: Laurent Vivier @ 2019-02-14 13:25 UTC (permalink / raw)
  To: Andreas Schwab, qemu-devel; +Cc: Riku Voipio

On 07/02/2019 15:51, Andreas Schwab wrote:
> System calls that return a socket address do so by writing the (possibly
> truncated) address into the provided buffer space, but setting the addrlen
> parameter to the actual size of the address.  To determine how much to
> copy back to the target memory the emulation needs to remember the old
> value of the addrlen parameter, so that it doesn't write past the buffer
> limits.
> 
> Signed-off-by: Andreas Schwab <schwab@suse.de>
> ---
>  linux-user/syscall.c | 38 ++++++++++++++++++++++----------------
>  1 file changed, 22 insertions(+), 16 deletions(-)

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH] linux-user: fix emulation of accept4/getpeername/getsockname/recvfrom syscalls
  2019-02-14 11:43   ` Andreas Schwab
@ 2019-02-14 13:27     ` Laurent Vivier
  0 siblings, 0 replies; 9+ messages in thread
From: Laurent Vivier @ 2019-02-14 13:27 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Riku Voipio, qemu-devel

On 14/02/2019 12:43, Andreas Schwab wrote:
> System calls that return a socket address do so by writing the (possibly
> truncated) address into the provided buffer space, but setting the
> addrlen parameter to the actual size of the address.  To determine how
> much to copy back to the target memory the emulation needs to remember
> the incoming value of the addrlen parameter, so that it doesn't write
> past the buffer limits.
> 
> Signed-off-by: Andreas Schwab <schwab@suse.de>
> ---
>  linux-user/syscall.c | 41 +++++++++++++++++++++++++----------------
>  1 file changed, 25 insertions(+), 16 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 4a320850bb..f24743a2b2 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -2885,7 +2885,7 @@ static abi_long do_sendrecvmmsg(int fd, abi_ulong target_msgvec,
>  static abi_long do_accept4(int fd, abi_ulong target_addr,
>                             abi_ulong target_addrlen_addr, int flags)
>  {
> -    socklen_t addrlen;
> +    socklen_t addrlen, ret_addrlen;
>      void *addr;
>      abi_long ret;
>      int host_flags;
> @@ -2909,11 +2909,13 @@ static abi_long do_accept4(int fd, abi_ulong target_addr,
>  
>      addr = alloca(addrlen);
>  
> -    ret = get_errno(safe_accept4(fd, addr, &addrlen, host_flags));
> +    ret_addrlen = addrlen;
> +    ret = get_errno(safe_accept4(fd, addr, &ret_addrlen, host_flags));
>      if (!is_error(ret)) {
> -        host_to_target_sockaddr(target_addr, addr, addrlen);
> -        if (put_user_u32(addrlen, target_addrlen_addr))
> +        host_to_target_sockaddr(target_addr, addr, MIN(addrlen, ret_addrlen));
> +        if (put_user_u32(ret_addrlen, target_addrlen_addr)) {
>              ret = -TARGET_EFAULT;
> +        }
>      }
>      return ret;
>  }
> @@ -2922,7 +2924,7 @@ static abi_long do_accept4(int fd, abi_ulong target_addr,
>  static abi_long do_getpeername(int fd, abi_ulong target_addr,
>                                 abi_ulong target_addrlen_addr)
>  {
> -    socklen_t addrlen;
> +    socklen_t addrlen, ret_addrlen;
>      void *addr;
>      abi_long ret;
>  
> @@ -2938,11 +2940,13 @@ static abi_long do_getpeername(int fd, abi_ulong target_addr,
>  
>      addr = alloca(addrlen);
>  
> -    ret = get_errno(getpeername(fd, addr, &addrlen));
> +    ret_addrlen = addrlen;
> +    ret = get_errno(getpeername(fd, addr, &ret_addrlen));
>      if (!is_error(ret)) {
> -        host_to_target_sockaddr(target_addr, addr, addrlen);
> -        if (put_user_u32(addrlen, target_addrlen_addr))
> +        host_to_target_sockaddr(target_addr, addr, MIN(addrlen, ret_addrlen));
> +        if (put_user_u32(ret_addrlen, target_addrlen_addr)) {
>              ret = -TARGET_EFAULT;
> +        }
>      }
>      return ret;
>  }
> @@ -2951,7 +2955,7 @@ static abi_long do_getpeername(int fd, abi_ulong target_addr,
>  static abi_long do_getsockname(int fd, abi_ulong target_addr,
>                                 abi_ulong target_addrlen_addr)
>  {
> -    socklen_t addrlen;
> +    socklen_t addrlen, ret_addrlen;
>      void *addr;
>      abi_long ret;
>  
> @@ -2967,11 +2971,13 @@ static abi_long do_getsockname(int fd, abi_ulong target_addr,
>  
>      addr = alloca(addrlen);
>  
> -    ret = get_errno(getsockname(fd, addr, &addrlen));
> +    ret_addrlen = addrlen;
> +    ret = get_errno(getsockname(fd, addr, &ret_addrlen));
>      if (!is_error(ret)) {
> -        host_to_target_sockaddr(target_addr, addr, addrlen);
> -        if (put_user_u32(addrlen, target_addrlen_addr))
> +        host_to_target_sockaddr(target_addr, addr, MIN(addrlen, ret_addrlen));
> +        if (put_user_u32(ret_addrlen, target_addrlen_addr)) {
>              ret = -TARGET_EFAULT;
> +        }
>      }
>      return ret;
>  }
> @@ -3043,7 +3049,7 @@ static abi_long do_recvfrom(int fd, abi_ulong msg, size_t len, int flags,
>                              abi_ulong target_addr,
>                              abi_ulong target_addrlen)
>  {
> -    socklen_t addrlen;
> +    socklen_t addrlen, ret_addrlen;
>      void *addr;
>      void *host_msg;
>      abi_long ret;
> @@ -3061,10 +3067,12 @@ static abi_long do_recvfrom(int fd, abi_ulong msg, size_t len, int flags,
>              goto fail;
>          }
>          addr = alloca(addrlen);
> +        ret_addrlen = addrlen;
>          ret = get_errno(safe_recvfrom(fd, host_msg, len, flags,
> -                                      addr, &addrlen));
> +                                      addr, &ret_addrlen));
>      } else {
>          addr = NULL; /* To keep compiler quiet.  */
> +        addrlen = 0; /* To keep compiler quiet.  */
>          ret = get_errno(safe_recvfrom(fd, host_msg, len, flags, NULL, 0));
>      }
>      if (!is_error(ret)) {
> @@ -3077,8 +3085,9 @@ static abi_long do_recvfrom(int fd, abi_ulong msg, size_t len, int flags,
>              }
>          }
>          if (target_addr) {
> -            host_to_target_sockaddr(target_addr, addr, addrlen);
> -            if (put_user_u32(addrlen, target_addrlen)) {
> +            host_to_target_sockaddr(target_addr, addr,
> +                                    MIN(addrlen, ret_addrlen));
> +            if (put_user_u32(ret_addrlen, target_addrlen)) {
>                  ret = -TARGET_EFAULT;
>                  goto fail;
>              }
> 

Applied to my linux-user branch.

Thanks,
Laurent

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH] linux-user: fix emulation of accept4/getpeername/getsockname/recvfrom syscalls
  2019-01-28 10:46 Andreas Schwab
@ 2019-01-31 18:07 ` no-reply
  0 siblings, 0 replies; 9+ messages in thread
From: no-reply @ 2019-01-31 18:07 UTC (permalink / raw)
  To: schwab; +Cc: fam, qemu-devel

Patchew URL: https://patchew.org/QEMU/mvmh8dt3vrp.fsf@suse.de/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PATCH] linux-user: fix emulation of accept4/getpeername/getsockname/recvfrom syscalls
Type: series
Message-id: mvmh8dt3vrp.fsf@suse.de

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
ca8c625f2b linux-user: fix emulation of accept4/getpeername/getsockname/recvfrom syscalls

=== OUTPUT BEGIN ===
ERROR: braces {} are necessary for all arms of this statement
#41: FILE: linux-user/syscall.c:2877:
+        if (put_user_u32(ret_addrlen, target_addrlen_addr))
[...]

ERROR: braces {} are necessary for all arms of this statement
#65: FILE: linux-user/syscall.c:2907:
+        if (put_user_u32(ret_addrlen, target_addrlen_addr))
[...]

ERROR: braces {} are necessary for all arms of this statement
#89: FILE: linux-user/syscall.c:2937:
+        if (put_user_u32(ret_addrlen, target_addrlen_addr))
[...]

total: 3 errors, 0 warnings, 98 lines checked

Commit ca8c625f2b00 (linux-user: fix emulation of accept4/getpeername/getsockname/recvfrom syscalls) has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/mvmh8dt3vrp.fsf@suse.de/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Qemu-devel] [PATCH] linux-user: fix emulation of accept4/getpeername/getsockname/recvfrom syscalls
@ 2019-01-28 10:46 Andreas Schwab
  2019-01-31 18:07 ` no-reply
  0 siblings, 1 reply; 9+ messages in thread
From: Andreas Schwab @ 2019-01-28 10:46 UTC (permalink / raw)
  To: qemu-devel

System calls that return a socket address do so by putting the possibly
truncated address into the provided buffer space, but setting the addrlen
parameter to the actual size of the address.  To determine how much to
copy back to the target memory the emulation needs to remember the old
value of the addrlen parameter, so that it doesn't write past the buffer
limits.

Signed-off-by: Andreas Schwab <schwab@suse.de>
---
 linux-user/syscall.c | 38 ++++++++++++++++++++++----------------
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index d5db01122aa4..46ca889556c8 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -2884,7 +2884,7 @@ static abi_long do_sendrecvmmsg(int fd, abi_ulong target_msgvec,
 static abi_long do_accept4(int fd, abi_ulong target_addr,
                            abi_ulong target_addrlen_addr, int flags)
 {
-    socklen_t addrlen;
+    socklen_t addrlen, ret_addrlen;
     void *addr;
     abi_long ret;
     int host_flags;
@@ -2908,10 +2908,11 @@ static abi_long do_accept4(int fd, abi_ulong target_addr,
 
     addr = alloca(addrlen);
 
-    ret = get_errno(safe_accept4(fd, addr, &addrlen, host_flags));
+    ret_addrlen = addrlen;
+    ret = get_errno(safe_accept4(fd, addr, &ret_addrlen, host_flags));
     if (!is_error(ret)) {
-        host_to_target_sockaddr(target_addr, addr, addrlen);
-        if (put_user_u32(addrlen, target_addrlen_addr))
+        host_to_target_sockaddr(target_addr, addr, MIN(addrlen, ret_addrlen));
+        if (put_user_u32(ret_addrlen, target_addrlen_addr))
             ret = -TARGET_EFAULT;
     }
     return ret;
@@ -2921,7 +2922,7 @@ static abi_long do_accept4(int fd, abi_ulong target_addr,
 static abi_long do_getpeername(int fd, abi_ulong target_addr,
                                abi_ulong target_addrlen_addr)
 {
-    socklen_t addrlen;
+    socklen_t addrlen, ret_addrlen;
     void *addr;
     abi_long ret;
 
@@ -2937,10 +2938,11 @@ static abi_long do_getpeername(int fd, abi_ulong target_addr,
 
     addr = alloca(addrlen);
 
-    ret = get_errno(getpeername(fd, addr, &addrlen));
+    ret_addrlen = addrlen;
+    ret = get_errno(getpeername(fd, addr, &ret_addrlen));
     if (!is_error(ret)) {
-        host_to_target_sockaddr(target_addr, addr, addrlen);
-        if (put_user_u32(addrlen, target_addrlen_addr))
+        host_to_target_sockaddr(target_addr, addr, MIN(addrlen, ret_addrlen));
+        if (put_user_u32(ret_addrlen, target_addrlen_addr))
             ret = -TARGET_EFAULT;
     }
     return ret;
@@ -2950,7 +2952,7 @@ static abi_long do_getpeername(int fd, abi_ulong target_addr,
 static abi_long do_getsockname(int fd, abi_ulong target_addr,
                                abi_ulong target_addrlen_addr)
 {
-    socklen_t addrlen;
+    socklen_t addrlen, ret_addrlen;
     void *addr;
     abi_long ret;
 
@@ -2966,10 +2968,11 @@ static abi_long do_getsockname(int fd, abi_ulong target_addr,
 
     addr = alloca(addrlen);
 
-    ret = get_errno(getsockname(fd, addr, &addrlen));
+    ret_addrlen = addrlen;
+    ret = get_errno(getsockname(fd, addr, &ret_addrlen));
     if (!is_error(ret)) {
-        host_to_target_sockaddr(target_addr, addr, addrlen);
-        if (put_user_u32(addrlen, target_addrlen_addr))
+        host_to_target_sockaddr(target_addr, addr, MIN(addrlen, ret_addrlen));
+        if (put_user_u32(ret_addrlen, target_addrlen_addr))
             ret = -TARGET_EFAULT;
     }
     return ret;
@@ -3042,7 +3045,7 @@ static abi_long do_recvfrom(int fd, abi_ulong msg, size_t len, int flags,
                             abi_ulong target_addr,
                             abi_ulong target_addrlen)
 {
-    socklen_t addrlen;
+    socklen_t addrlen, ret_addrlen;
     void *addr;
     void *host_msg;
     abi_long ret;
@@ -3060,10 +3063,12 @@ static abi_long do_recvfrom(int fd, abi_ulong msg, size_t len, int flags,
             goto fail;
         }
         addr = alloca(addrlen);
+        ret_addrlen = addrlen;
         ret = get_errno(safe_recvfrom(fd, host_msg, len, flags,
-                                      addr, &addrlen));
+                                      addr, &ret_addrlen));
     } else {
         addr = NULL; /* To keep compiler quiet.  */
+        addrlen = 0; /* To keep compiler quiet.  */
         ret = get_errno(safe_recvfrom(fd, host_msg, len, flags, NULL, 0));
     }
     if (!is_error(ret)) {
@@ -3076,8 +3081,9 @@ static abi_long do_recvfrom(int fd, abi_ulong msg, size_t len, int flags,
             }
         }
         if (target_addr) {
-            host_to_target_sockaddr(target_addr, addr, addrlen);
-            if (put_user_u32(addrlen, target_addrlen)) {
+            host_to_target_sockaddr(target_addr, addr,
+                                    MIN(addrlen, ret_addrlen));
+            if (put_user_u32(ret_addrlen, target_addrlen)) {
                 ret = -TARGET_EFAULT;
                 goto fail;
             }
-- 
2.20.1

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-02-14 13:27 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-07 14:51 [Qemu-devel] [PATCH] linux-user: fix emulation of accept4/getpeername/getsockname/recvfrom syscalls Andreas Schwab
2019-02-07 15:22 ` no-reply
2019-02-07 15:26 ` no-reply
2019-02-14  9:17 ` Laurent Vivier
2019-02-14 11:43   ` Andreas Schwab
2019-02-14 13:27     ` Laurent Vivier
2019-02-14 13:25 ` Laurent Vivier
  -- strict thread matches above, loose matches on Subject: below --
2019-01-28 10:46 Andreas Schwab
2019-01-31 18:07 ` no-reply

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.