Types of several members of both struct cmsghdr and struct msghdr are different from those specified in the standard. The patches attached below fix this issue without breaking the compatibility with the linux kernel.
Created attachment 7591 [details] Patch to fix cmsghdr member type
Created attachment 7592 [details] Patch to fix struct msghdr members types
Padding should use __glibc_reserved* names. To make part of an existing field into padding like this, the following need to be true: * If the kernel reads the field, all existing kernel versions supported by glibc already treat that part of the field as padding, so they will not misinterpret uninitialized data there. If that's not the case, glibc needs to initialize the padding appropriately before passing the structure to the kernel. * If the kernel writes the field, it gives an overflow error rather than treating that part of the field as something to which a significant value may be written. If it treats it as significant, glibc needs to check in userspace for overflow and set an error in that case (just as the asm-generic implementations of 32-bit stat functions do). * If user programs write the field, so that for old programs the high bits are significant but for new programs the high bits are padding, symbol versioning needs to be used to avoid misinterpreting values written there by existing binaries. * If user programs read the field, so that old programs may interpret high bits written by glibc or the kernel as significant, those bits need initializing appropriately for the existing programs.
(In reply to joseph@codesourcery.com from comment #3) > Padding should use __glibc_reserved* names. > > To make part of an existing field into padding like this, the following > need to be true: > > * If the kernel reads the field, all existing kernel versions supported by > glibc already treat that part of the field as padding, so they will not > misinterpret uninitialized data there. If that's not the case, glibc > needs to initialize the padding appropriately before passing the structure > to the kernel. > > * If the kernel writes the field, it gives an overflow error rather than > treating that part of the field as something to which a significant value > may be written. If it treats it as significant, glibc needs to check in > userspace for overflow and set an error in that case (just as the > asm-generic implementations of 32-bit stat functions do). > > * If user programs write the field, so that for old programs the high bits > are significant but for new programs the high bits are padding, symbol > versioning needs to be used to avoid misinterpreting values written there > by existing binaries. > > * If user programs read the field, so that old programs may interpret high > bits written by glibc or the kernel as significant, those bits need > initializing appropriately for the existing programs. Thanks for the tip. I should create appropriate __libc_{sendmsg,recvmsg} functions for that, am I right?
On Wed, 7 May 2014, sickmind at i2pmail dot org wrote: > Thanks for the tip. I should create appropriate __libc_{sendmsg,recvmsg} > functions for that, am I right? Exactly what's appropriate depends on the details of the interfaces - what libc interfaces use these structures as inputs, outputs or both, and what kernel interfaces libc uses with them (and again, whether as inputs, outputs or both), and how the kernel handles the high bits in question. The patch submission (to libc-alpha, see the contribution checklist on the wiki) will need to contain a detailed analysis of those issues that justifies why the changes in the patch don't cause any problems for existing binaries or cause the kernel interface to be used incorrectly. If you write up that analysis before implementing the patch, it may help people advise on what should or should not be in the patch.
Fixing this issue is a lot harder than it looks, especially since the kernel folks went and added sendmmsg/recvmmsg that also use these structures. For both recvmmsg and sendmmsg the msghdr structures lie in memory that's (by the interface contract) writable by the function so it's easy to just patch zeros into the padding slots before making the syscall. However sendmmsg's msghdr points to an already-filled control buffer containing cmsghdr structures that may contain junk in their padding areas, and this buffer is NOT writable. So you have to do something like copying the control buffers (which may be of very large total length) to a copy, modifying the msghdr structures to point to the copies, making the syscall, then putting it all back. At that point it might be easier (this is what we do in musl) to just loop calling sendmsg rather than bothering with the sendmmsg syscall... We should really just demand that this be fixed on the kernel side, so that when glibc drops support for old kernels somewhere down the line, these ugly hacks can be dropped and the syscalls can just be safely usable directly.
(In reply to Rich Felker from comment #6) > We should really just demand that this be fixed on the kernel side, so that > when glibc drops support for old kernels somewhere down the line, these ugly > hacks can be dropped and the syscalls can just be safely usable directly. What about the usual kernel devs' motto about not breaking the userspace?
I don't think fixing the kernel to ignore the upper 32 bits of these wrongly-64-bit fields would "break userspace". It would mask bugs in old applications that wrote huge (>0xffffffff) values to the fields and expected to get an error from doing so rather than for the high bits to be ignored, but I really doubt any such applications exist. For all applications using the interfaces correctly, old apps with the wrong struct definition would already be storing zeros in the high bits, and new applications with the right struct definition would be leaving junk in the upper bits for the kernel (or, for the time being, glibc when it does the fixup in userspace to support old kernels) to ignore.
*** Bug 18233 has been marked as a duplicate of this bug. ***
After some discussion in libc-alpha about this POSIX compliance fix, I see that GLIBC should indeed revert back to previous definition of msghdr and cmsghdr and implementation of sendmsg, recvmsg, sendmmsg, recvmmsg due some reasons: * The possible issue where the syscalls wrapper add the compatibility layer is quite limited in scope and range. And kernel current also add some limits to the values on the internal msghdr and cmsghdr fields: - msghdr::msg_iovlen larger than UIO_MAXIOV (1024) returns EMSGSIZE. - msghdr::msg_controllen larger than INT_MAX returns ENOBUFS. * There is a small performance hit for recvmsg/sendmsg/recmmsg which is neglectable, but it is a big hit for sendmmsg since now instead of calling the syscall for the packed structure, GLIBC is calling multiple sendmsg. This defeat the very existence of the syscall. * It currently breaks libsanitizer build on GCC [1] (I fixed on compiler-rt). However the fix is incomplete because it does add any runtime check since libsanitizer currently does not have any facility to intercept symbols with multiple version [2]. This, along with incorret dlsym/dlvsym return for versioned symbol due another bug [3], makes hard to interpose versioned symbols. Also, current approach of fixing GCC PR#71445 leads to half-baked solutions without versioned symbol interposing. This patch basically reverts commits 2f0dc39029ae08, 222c2d7f4357d66, af7f7c7ec8dea1. I decided to not revert abf29edd4a3918 (Adjust kernel-features.h defaults for recvmsg and sendmsg) mainly because it does not really address the POSIX compliance original issue and also adds some cleanups. This is not a state to mark this bug a WONTFIX.
Do we still plan to fix this bug? I'm inclined to close it as WONTFIX.
(In reply to Florian Weimer from comment #11) > Do we still plan to fix this bug? I'm inclined to close it as WONTFIX. The fix it self it just a matter to adapt 2f0dc39029ae08, 222c2d7f4357d66, af7f7c7ec8dea1 to master. However to proper fix I think we should first revise and fix dlsym/dlvsym issues (BZ#1319, BZ#12977, BZ#14932) so libraries that override libc symbols (like sanitizers) can access correctly all the versions. In any case, I think the overall gain on fix it is quite small and I think it would be ok to just close it.
It would be really disappointing to see this closed. glibc has made a lot of progress on not treating this kind of issue as a joke and I feel like this would be a setback.
(In reply to Rich Felker from comment #13) > It would be really disappointing to see this closed. glibc has made a lot of > progress on not treating this kind of issue as a joke and I feel like this > would be a setback. It's a kernel bug. I consider rewriting the ancillary data too risky. The kernel ABI around recvmsg/sendmsg isn't totally stable, too.
It is a kernel bug, but there are A LOT of kernel bugs that are conformance bugs where userspace (libc) has to patch around the kernel behavior to get a conforming environment. Things like wrong errno values, spurious failure in a corner case that should be success or vice versa, etc. Maybe this is getting off-topic for an individual bug, but "WONTFIX" has a really bad reputation and implication of finality. If it's really too risky to fix now or in the forseeable future, it would be nice to have a state expressing that an issue is blocked (has no current plausible path forward) or deferred without an explicit and willful "intent not to fix".
On Wed, 24 Oct 2018, bugdal at aerifal dot cx wrote: > Maybe this is getting off-topic for an individual bug, but "WONTFIX" has a > really bad reputation and implication of finality. If it's really too risky to > fix now or in the forseeable future, it would be nice to have a state > expressing that an issue is blocked (has no current plausible path forward) or > deferred without an explicit and willful "intent not to fix". I agree WONTFIX should be avoided except for bugs in removed functionality (e.g. we remove a port which has some open port-specific bugs). If something depends on an external fix (kernel, compiler, standard DR resolution, etc.), SUSPENDED is the appropriate state (WAITING is different, it means waiting for more information from the submitter). If we reach consensus that something is not a bug and the change requested by the submitter is not desirable, INVALID should be used instead of WONTFIX.
The master branch has been updated by Samuel Thibault <sthibaul@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=7647d1901ea2b34fafd95ecddf52905a3d314368 commit 7647d1901ea2b34fafd95ecddf52905a3d314368 Author: Samuel Thibault <samuel.thibault@ens-lyon.org> Date: Mon May 1 14:48:06 2023 +0200 socket: Fix tst-cmsghdr-skeleton.c use of cmsg_len cmsg_len is supposed to be socklen_t according to standards, but it was made size_t on Linux, see BZ 16919. For ports that have it socklen_t, SIZE_MAX is too large. We can however explicitly cast it to the type of cmsg_len so it will fit according to that type.