[Qemu-devel] [PATCH] linux-user: Fix payload size logic in host_to_target_cmsg()

[Qemu-devel] [PATCH] linux-user: Fix payload size logic in host_to_target_cmsg()

[Peter Maydell]

Coverity points out that there's a missing break in the switch in
host_to_target_cmsg() where we update tgt_len for
cmsg_level/cmsg_type combinations which require a different length
for host and target (CID 1385425).  To avoid duplicating the default
case (target length same as host) in both switches, set that before
the switch so that only the cases which want to override it need any
code.

This fixes a bug where we would have used the wrong length
for SOL_SOCKET/SO_TIMESTAMP messages where the target and
host have differently sized 'struct timeval' (ie one is 32
bit and the other is 64 bit).

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/syscall.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index af8603f1b7..88d166cdff 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -1825,6 +1825,7 @@ static inline abi_long host_to_target_cmsg(struct target_msghdr *target_msgh,
         /* Payload types which need a different size of payload on
          * the target must adjust tgt_len here.
          */
+        tgt_len = len;
         switch (cmsg->cmsg_level) {
         case SOL_SOCKET:
             switch (cmsg->cmsg_type) {
@@ -1834,8 +1835,8 @@ static inline abi_long host_to_target_cmsg(struct target_msghdr *target_msgh,
             default:
                 break;
             }
+            break;
         default:
-            tgt_len = len;
             break;
         }
 
-- 
2.17.0

Re: [Qemu-devel] [PATCH] linux-user: Fix payload size logic in host_to_target_cmsg()

[Laurent Vivier]

Le 18/05/2018 à 20:47, Peter Maydell a écrit :
> Coverity points out that there's a missing break in the switch in
> host_to_target_cmsg() where we update tgt_len for
> cmsg_level/cmsg_type combinations which require a different length
> for host and target (CID 1385425).  To avoid duplicating the default
> case (target length same as host) in both switches, set that before
> the switch so that only the cases which want to override it need any
> code.
> 
> This fixes a bug where we would have used the wrong length
> for SOL_SOCKET/SO_TIMESTAMP messages where the target and
> host have differently sized 'struct timeval' (ie one is 32
> bit and the other is 64 bit).
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  linux-user/syscall.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index af8603f1b7..88d166cdff 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -1825,6 +1825,7 @@ static inline abi_long host_to_target_cmsg(struct target_msghdr *target_msgh,
>          /* Payload types which need a different size of payload on
>           * the target must adjust tgt_len here.
>           */
> +        tgt_len = len;
>          switch (cmsg->cmsg_level) {
>          case SOL_SOCKET:
>              switch (cmsg->cmsg_type) {
> @@ -1834,8 +1835,8 @@ static inline abi_long host_to_target_cmsg(struct target_msghdr *target_msgh,
>              default:
>                  break;
>              }
> +            break;
>          default:
> -            tgt_len = len;
>              break;
>          }
>  
> 

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