Doing a ping for this patch. https://patchew.org/QEMU/20210516091536.1042693-1-ke...@lithdew.net/
Best regards, Kenta Iwasaki On Sun, 16 May 2021 at 21:57, Kenta Iwasaki <ke...@lithdew.net> wrote: > 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 <laur...@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 <ke...@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 >> >
