From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,MENTIONS_GIT_HOSTING,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D6FA6C433ED for ; Sun, 16 May 2021 13:59:50 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id CEB976113E for ; Sun, 16 May 2021 13:59:49 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org CEB976113E Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=lithdew.net Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:58518 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1liHJ6-0007z4-L3 for qemu-devel@archiver.kernel.org; Sun, 16 May 2021 09:59:48 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:47526) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1liGLS-00085U-FQ for qemu-devel@nongnu.org; Sun, 16 May 2021 08:58:10 -0400 Received: from mail-vs1-xe34.google.com ([2607:f8b0:4864:20::e34]:40574) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1liGLQ-0004MI-2D for qemu-devel@nongnu.org; Sun, 16 May 2021 08:58:10 -0400 Received: by mail-vs1-xe34.google.com with SMTP id o192so1794132vsd.7 for ; Sun, 16 May 2021 05:58:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=lithdew-net.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=ifyi6HTBvBxrmElLxlfoUzK6bVdYCAE7BBef6VbPxOI=; b=WdWFQIJafNzzR+BNMd4Rynv1Dye3FSv7i4o+8RKwdZOEaswhSWCDExMzMg5K664DH+ o0VbLsl/4+C8yO5EdgxiTOon+Aa6GaTRyQ+zHUtm0nYEUnVCqm5kSD6WQpB1CSQPz/5r uXFOjrqRfZecWsPjCwdQ6mcV3OORON5bxJ26TH/uZtkXYKhlXEk+AtOech6AOXx6f/ZB juKe5RNfjWoMfAIjN0NcNV9Q3Isc3KkjZ85Ndd2DZNUWU79sGD+ryEH3zO1yDxGmxq6/ lufRRtVVjd+amK/0Cc5HSIO8FwND219YxQSycKplh1p5vSUJmkfPhJkDN2GeaL6neX8V 8Pgw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=ifyi6HTBvBxrmElLxlfoUzK6bVdYCAE7BBef6VbPxOI=; b=oC4gdVoYa1jVha81++U24qd44a+NWBbc3GaGN8xfF/Fm7Tcx9vB/o4t6m/nQXemOBc dx+NuY56rWIZ0xB76ANo8ZH6fGaxVQdQAqXKBazQLKgkobe4+75uN4owRjIFWLm2y/Wa 0SqDGRMzXhSVow9aB2boUh1OLDeTszcg4EMODl3xYaSF0cFJPjK/yitUpWTlyjr01HqO Wi1xocvQqUh0JMGSJlf39QLDwkLh6AzkrQabhaHDh9ts8r0ZGJKPGixVV3qAJR9L0esi vFRgASBnXnCSKXbY919yx7be3Losw8TjT9I94TPbvvanZVHOtzcFckRRF/sSHz68VefU p/Hw== X-Gm-Message-State: AOAM5321JjSITg6smlLL9ONxRNo3YZAfv4ef5ews8P+7smY8fRpBPRxw ZufCQivPIZYFRkZ5mObk5n83UTen4+qC9g8FDqE1GUD2NTJYcXeP5lQ= X-Google-Smtp-Source: ABdhPJyrKdPg08ObZcX8UV7C2uq/b97d4wJVj9zhim4MI4uLIA0aMvO2apiojGFG2dDGIc9NbLKxHoeA9Wum+lwR+wY= X-Received: by 2002:a67:64c5:: with SMTP id y188mr46334028vsb.19.1621169886835; Sun, 16 May 2021 05:58:06 -0700 (PDT) MIME-Version: 1.0 References: <20210516091536.1042693-1-kenta@lithdew.net> In-Reply-To: From: Kenta Iwasaki Date: Sun, 16 May 2021 21:57:55 +0900 Message-ID: Subject: Re: [PATCH] linux-user/syscall: zero-init msghdr in do_sendrecvmsg_locked To: Laurent Vivier Content-Type: multipart/alternative; boundary="00000000000039c81705c27207c9" Received-SPF: none client-ip=2607:f8b0:4864:20::e34; envelope-from=kenta@lithdew.net; helo=mail-vs1-xe34.google.com X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_NONE=0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-Mailman-Approved-At: Sun, 16 May 2021 09:58:24 -0400 X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: qemu-devel@nongnu.org Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" --00000000000039c81705c27207c9 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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-71839= 2360 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=3D7168790763cdeb794df52be6e3b= 39fbb021c5a64 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 wrote: > Le 16/05/2021 =C3=A0 11:15, Kenta Iwasaki a =C3=A9crit : > > 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 > > --- > > 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 =3D { 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 > --00000000000039c81705c27207c9 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Sure,

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

1. require specializing code = in do_sendrecvmsg_locked() for each individual libc and kernel version, or<= /div>
2. zeroing out all bytes of `msghdr`, b/c certain libc or kernel = versions may misinterpret the undefined padding bytes that come from misali= gnment in the struct as actual syscall params.

The= patch I provided would be going for route #2, given that it's a simple= r 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 speci= fied the layout of `msghdr` differently from POSIX. Given that this implies= incompatibility between kernels on how `msghdr` is specified, different li= bc 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, an= d the Linux kernel: https://github.com/void-linux/void-pac= kages/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 lay= ing out `msghdr` against the POSIX standard: http= ://git.musl-libc.org/cgit/musl/commit/?id=3D7168790763cdeb794df52be6e3b39fb= b021c5a64

Also, here is a link to the `msghdr`= struct layout in musl libc's bleeding edge branch as of now:=C2=A0h= ttps://git.musl-libc.org/cgit/musl/tree/include/sys/socket.h#n22
<= div>
As for my rationale for sending in this patch, it is bec= ause I'm currently implementing cross-platform networking in the standa= rd library for the Zig programming language, and have run into this exact s= ame problem with EMSGSIZE being returned by sendmsg() when tests are run th= rough QEMU on x86_64-linux-musl.

My discussions wi= th the Zig community about it alongside debug logs regarding the exact para= meters being fed to the sendmsg() syscall can be found here:=C2=A0https:= //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 provid= e 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 =C3= =A0 11:15, Kenta Iwasaki a =C3=A9crit=C2=A0:
> The mixing of libc and kernel versions of the layout of the `msghdr` > struct causes EMSGSIZE to be returned by sendmsg if the `msghdr` struc= t
> 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>
> ---
>=C2=A0 linux-user/syscall.c | 2 +-
>=C2=A0 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, st= ruct target_msghdr *msgp,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 int f= lags, int send)
>=C2=A0 {
>=C2=A0 =C2=A0 =C2=A0 abi_long ret, len;
> -=C2=A0 =C2=A0 struct msghdr msg;
> +=C2=A0 =C2=A0 struct msghdr msg =3D { 0 };
>=C2=A0 =C2=A0 =C2=A0 abi_ulong count;
>=C2=A0 =C2=A0 =C2=A0 struct iovec *vec;
>=C2=A0 =C2=A0 =C2=A0 abi_ulong target_vec;
>

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

Could you explain more?

Thanks,
Laurent
--00000000000039c81705c27207c9--