From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/14069 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Norbert Lange Newsgroups: gmane.linux.lib.musl.general Subject: Re: use of varargs in open and various functions Date: Thu, 11 Apr 2019 18:03:16 +0200 Message-ID: Reply-To: musl@lists.openwall.com Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="100795"; mail-complaints-to="usenet@blaine.gmane.org" To: musl@lists.openwall.com Original-X-From: musl-return-14085-gllmg-musl=m.gmane.org@lists.openwall.com Thu Apr 11 18:03:49 2019 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by blaine.gmane.org with smtp (Exim 4.89) (envelope-from ) id 1hEcB2-000Q0t-5d for gllmg-musl@m.gmane.org; Thu, 11 Apr 2019 18:03:48 +0200 Original-Received: (qmail 3407 invoked by uid 550); 11 Apr 2019 16:03:40 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 3324 invoked from network); 11 Apr 2019 16:03:39 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:from:date:message-id:subject:to; bh=Wi4lgJJpXBSSo3Q5cqqD90+oh8Dnx9k5cpe0OZ/MxhY=; b=C6CbQ40RCQy7+VsmzqCFBdUulZZR/tjI+kKx66NCxHNRx4jSL6jbIfVek6UKBTRghK +JbggsKSuOKJIYt4wyGcUt0e9764jrGI2kHFfOOKymj4gcSObUr5ECl4aJpgVKPWPxlY +UQAAUzWkjHf38aKTmOajJzn149GnHyOo/WPiyJxGO6HaQZ3aFhF5sEerhWnEJXXaO9Z zOsTsMJBxpkCpvruLZw57pFcDEuDj0Yla3tpKqlvdg8IUW+8X4dOOxj4WN6+glMT5suL ZYzpsyp/nLehKbAs+vZ4Yq3bNwxptWLha2xRDrREok1Vw6yqKhwpYrqmoIUaJS3ed2df kILA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:from:date:message-id:subject:to; bh=Wi4lgJJpXBSSo3Q5cqqD90+oh8Dnx9k5cpe0OZ/MxhY=; b=mQSe29/yVzFDnlAFU1pAQLRskWbwDU5s6d60IETZgPkfzOCjdhYuCbw9Q919AtoN9E SiLlEBsY3PPWjtJ4SBnSg4RKnODAH7BIeanki7Iz8rvPqBwNRydnMEDfrBBYT9Dybua7 0pcyOiB+rEejiKj/pys+ANwek7yfokRkTGK04zdJjhwGDgz3q3wrG1bUUw5D8ZAbksXJ qpwQ5CrdFi2X9D2TGv/2N9Sl2jTy4VRIIQNJFnVm7WJLDIRXcgOadAdoJbdddf/rxguV hVPL0At6JdpccTwtDeOO417FJHVNuebDeEneLsRShrBAjox+Gx+fH/PMklu+LbYQMSsQ UVnQ== X-Gm-Message-State: APjAAAW7L5m5syS42wQrIB3ozT4+NjqVQooN2PYOH0IRsqDy3kIKB48X RGq3E5cUJDV/m8aSV5Q4c2HLP8GGOLOBPWILYAfBm7GD X-Google-Smtp-Source: APXvYqwz/RVFELtg/uvfs98MEzOr390AB+3nt1GxD1By5h76mKtLx+PSmEQ2y9biXIZOxhMfl+S3kDcTxZwqTbOKmMY= X-Received: by 2002:a24:3a8b:: with SMTP id m133mr8999597itm.146.1554998607436; Thu, 11 Apr 2019 09:03:27 -0700 (PDT) Xref: news.gmane.org gmane.linux.lib.musl.general:14069 Archived-At: > > On Thu, Apr 11, 2019 at 04:25:50PM +0200, Norbert Lange wrote: > > Hello, > > > > I had some dealings with software that forwards arguments from vararg functions, > > the kind with optional but known fixed type. > > open, fcntl, ioctl are some examples. > > > > What happens in musl is that the optional argument is optionally or > > always retrieved (fcntl). > > I think this is pretty much pointless, the only reason to use varags would be: > > > > 1. varying argument types (plainly not possibly to replace with > > direct arguments) > > 2. different parameter passing than direct arguments (ABI compatibility) > > 3. accessing parameters the caller has not provided. > > > > 1. disqualifies functions like printf which I am not planning to touch. > > 2. would need more tests, but I expect this to be true for all modern > > architectures, > It's not true. Not even for x86_64. If you declared open or fcntl > wrong, the caller would not set rax indicating lack of sse args, and > when calling into libc.so built with the correct variadic definitions, > bogus valid in rax could crash or cause runaway wrong code execution. > It's also not true on powerpc, at least for aggregate types -- see > commit 2b47a7aff24bbfbe7ba89fc6d542acc9f5493ae2. declaration would stay the same, on x86_64 the caller will set rax. The implementation of open/fcntl would have a different declaration. if ppc does something similar, would this matter if the caller calls a vararg function with a implementation not using varargs? For now consider that callers always use the vararg declaration. > Perhaps more importantly, it's completely untrue for the abstract > machine and advanced linking and symbolic execution models (LTO, tools > like Frama-C) where mismatched function type would just be a hard > linking error. That's more of an argument, the patch I added is just one example, you could use some barriers like implementing an another_open, and then substitute this as symbol for "open" (alias attribute or linker magic). Or just disable LTO for these functions. > > > 3. thats a thing for open where the 3rd argument is only accessed if > > certain flags are set, > > but most other functions in musl seem to always access "optional" arguments. > There are only a few which do this, and it's a bug, but possibly an > unfixable bug. For instance syscall() can't really know how many to > access without a huge table, and even then it's sometimes "valid" to > call a syscall with fewer args depending on flags or context. So we > really probably need a stupid asm wrapper-barrier here to "launder" > the mismatched state (converting missing args abstractly into args > with indeterminate value). at which point LTO and analysis tools will run against an barrier aswell (not that much lost at syscall wrappers IMHO). > > reading theses non-existing arguments could trigger tools like > > valgrind, but other > > than that it should not be an issue aslong a couple bytes of stack > > is available. > > > > I tested code calling the functions, I believe calls are identical for > > varags and normal functions, > > for all architectures that are supported by musl. [1] > > (sidenote: clang generates absolutely awful code for those varag functions). > > > > So, is there any reason not to make this the default for musl (adding > > a fallback option via a define)? I am running a few tools (bash, nano) > > compiled with the applied patch and so far I have no adverse effects, > > it should not affect ABI for machines satisfying 2. and it gets rid of > > some wacky code that in the end just passes in another register (like > > open). > > > > Further I would like to add that Torvalds shares a similar view [2]. > A lot of effort was spent making this right; it's not going to be > reversed. Torvalds is wrong about lots of things; a large part of > what's nontrivial in musl has been working around things he's wrong > about. :-) I am not implying he is always right. > What benefit are you trying to get from this? Just some size > reduction? I ran into this problem with Xenomai Userspace, which "wraps" a subset of libc and posix functions to allow using a hard realtime subsystem with "normal" posix code (and transparent fallback to the regular system). Except the wrapper for open missed the special case of O_TMPFILE, hence me fixing that and looking at "prior art". open could have been simple function with 3 parameters, just doing a syscall, instead the delicate vararg hacks spreading everywhere. So the benefit I would get from that is getting rid of some weirdness very few people know about. And curiosity why it has to be this way in case that's not possible. > I assumed GCC would be non-idiotic and generate exactly the > same code for the correct version with conditional va_arg and the > incorrect non-variadic version, but apparently it's doing gratuitous > store/load to/from stack, at least on the versions I've tested. If > this bothers you, please pester the GCC folks to fix it. (Note: GCC > can and does optimize this out when inlining variadic functions, so > the high-level machinery is there.) Yeah, thats a sidenote. I added a comment on the bugtracker for clang/llvm which seems to have a crude bug there. Its still better to keep things as simple as reasonable. > > [1] https://godbolt.org/z/asBT92 > > [2] https://lkml.org/lkml/2014/10/30/812 > > > > PS. why is this a thing in open: > > int fd = __sys_open_cp(filename, flags, mode); > > if (fd>=0 && (flags & O_CLOEXEC)) > > __syscall(SYS_fcntl, fd, F_SETFD, FD_CLOEXEC); > > > > Is this to support old kernels? > > I thought O_CLOEXEC is used to fight races during a fork, > > so if its not supported by the kernel it wont do alot. > There were a few kernel versions, probably no longer relevant because > it was quickly fixed IIRC and they weren't LTS versions, where > O_CLOEXEC was ignored completely. The result was that even in > single-threaded programs where there was no race, you had a huge fd > leak. The workaround here was to mitigate the badness of that. If > someone wants to spend a little effort researching this, I think we'd > find that we could remove the workaround now. > Rich To me that sounds like some min required kernel version, would make a useful configure option. Norbert