Re: [PATCH] linux-user/syscall: zero-init msghdr in do_sendrecvmsg_locked

[Kenta Iwasaki]

Sure,

The bytes of `msghdr` need to be cleared because the `msghdr` struct layout specified in QEMU appears to generalize between the definitions of `msghdr` across different libc's and kernels. To appropriately generalize `msghdr` across libc's and kernels would either:

1. require specializing code in do_sendrecvmsg_locked() for each individual libc and kernel version, or

2. zeroing out all bytes of `msghdr`, b/c certain libc or kernel versions may misinterpret the undefined padding bytes that come from misalignment in the struct as actual syscall params.

The patch I provided would be going for route #2, given that it's a simpler fix for the underlying problem for the short term.

What I believe is the background behind why the struct layout has been a problem is because, since the beginning, the Linux kernel has always specified the layout of `msghdr` differently from POSIX. Given that this implies incompatibility between kernels on how `msghdr` is specified, different libc projects such as musl and glibc provide different workarounds by laying out `msghdr` differently amongst one another.

A few projects running tests/applications through QEMU have been bitten by this, and a solution that one of the projects discovered was that patching QEMU to zero-initialize the bytes msghdr the same way my patch does allow for compatibility between different `msghdr` layouts across glibc, musl, and the Linux kernel: https://github.com/void-linux/void-packages/issues/23557#issuecomment-718392360

For some additional useful context, here's a link pointing changes musl libc made to laying out `msghdr` b/c of Linux incorrectly laying out `msghdr` against the POSIX standard: http://git.musl-libc.org/cgit/musl/commit/?id=7168790763cdeb794df52be6e3b39fbb021c5a64

Also, here is a link to the `msghdr` struct layout in musl libc's bleeding edge branch as of now: https://git.musl-libc.org/cgit/musl/tree/include/sys/socket.h#n22

As for my rationale for sending in this patch, it is because I'm currently implementing cross-platform networking in the standard library for the Zig programming language, and have run into this exact same problem with EMSGSIZE being returned by sendmsg() when tests are run through QEMU on x86_64-linux-musl.

My discussions with the Zig community about it alongside debug logs regarding the exact parameters being fed to the sendmsg() syscall can be found here: https://github.com/ziglang/zig/pull/8750#issuecomment-841641576

Hope this gives enough context about the problem and patch, but please do let me know if there is any more information that I could provide which would help.

Best regards,

Kenta Iwasaki

On Sun, 16 May 2021 at 19:53, Laurent Vivier <laurent@vivier.eu> wrote:

Le 16/05/2021 à 11:15, Kenta Iwasaki a écrit :
> The mixing of libc and kernel versions of the layout of the `msghdr`
> struct causes EMSGSIZE to be returned by sendmsg if the `msghdr` struct
> is not zero-initialized (such that padding bytes comprise of
> uninitialized memory).
>
> Other parts of the QEMU codebase appear to zero-initialize the `msghdr`
> struct to workaround these struct layout issues, except for
> do_sendrecvmsg_locked in linux-user/syscall.c.
>
> This patch zero-initializes the `msghdr` struct in
> do_sendrecvmsg_locked.
>
> Signed-off-by: Kenta Iwasaki <kenta@lithdew.net>
> ---
>  linux-user/syscall.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 95d79ddc43..f60b7e04d5 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -3337,7 +3337,7 @@ static abi_long do_sendrecvmsg_locked(int fd, struct target_msghdr *msgp,
>                                        int flags, int send)
>  {
>      abi_long ret, len;
> -    struct msghdr msg;
> +    struct msghdr msg = { 0 };
>      abi_ulong count;
>      struct iovec *vec;
>      abi_ulong target_vec;
>

It seems do_sendrecvmsg_locked() initializes all the fields of the structure, I don't see why we
need to clear it before use.

Could you explain more?

Thanks,
Laurent