[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <CADYdroPbnSrOPNKa5Htf7Mw0oRh4JRYYnE2tFtnsa+wrDcqa8Q@mail.gmail.com>
Date: Thu, 11 Apr 2019 18:36:31 +0200
From: Norbert Lange <nolange79@...il.com>
To: musl@...ts.openwall.com
Subject: Re: use of varargs in open and various functions
> 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,
> Tough. The interfaces are just defined this way. In case of fcntl, it
> is merely a convenience of circumstance that allows musl to do what it
> does. fcntl()'s third argument can be a pointer to various structures,
> or an unsigned long. Linux requires a C implementation where pointers
> can be losslessly converted to long and back, but that is the only
> reason this call to va_arg() works.
Those are C interfaces, they can be implemented in any language you
you see fit.
if you write asm, you can just pick the registers or pull the values from
the stack.
declaration can and should stay the same, no point stressing this further.
> > the only reason to use varags would be:
> >
> > 1. varying argument types (plainly not possibly to replace with
> > direct arguments)
> That is done in fcntl(). That last arg can be a pointer to struct flock,
> or it can be an int, or it can be nothing.
> Ooh, Rich, someone found UB! If fcntl() is called with one of the codes
> requiring no argument (such as F_GETFD), then the va_arg() in there
> tries to pop an argument off an empty argument list. Which is UB, if I'm
> not mistaken. In fact, the patch for open() to avoid exactly this
> problem happened not so long ago, right?
UB can still be deterministic. Just as longs have the same size in linux,
some assumptions could be made for a c library targeting linux.
dlsym further assumes void * and void (*)() have the same size.
>
> Now for the master class: Are there ioctl() codes that also require no
> argument?
> > 2. different parameter passing than direct arguments (ABI compatibility)
> I have worked with ABIs like this before. Granted, not on Linux, but
> they do exist. I don't think anyone here wants to introduce a dependency
> on an ABI definition.
Would be debatable to me (I know I have no say in the decission),
it certainly would restrict the scope of the project.
> > 3. accessing parameters the caller has not provided.
> >
> Well, that is probably UB, but maybe you just worded it badly.
yes, just as accessing that same parameter via a vararg,
which musl does in several occasions. the manpage of the functions
might not define a call without argument, but the C declaration certainly does.
(practically it will be garbage from a register or stack.)
>
> > 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,
> > atleast its true for most of them.
> What is true of modern architectures? Also, no testing required,
> parameter passing is defined in the relevant ABI documents.
> > 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.
> > 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.
> >
> In practice, I would expect such a va_arg() call to return an
> arbitrary value. But yeah, it is UB, so anything *might* happen.
> > 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]
> Ah, "I tested it and it works for me". There are a few people here for
> whom that will not be good enough.
Shees. means I did not dig through ABI docs, but looked at
what compilers are spitting out.
> > (sidenote: clang generates absolutely awful code for those varag functions).
> >
> Well, then maybe they need to tighten up their vararg codegen.
> > 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).
> >
> Yes, and a very simple one: UB. open() has been declared with an
> ellipsis for ages now, and in fact this has made it into POSIX[1]. So we
> can't change the declaration in the header file. Defining the function
> differently from its declaration will result in a compiler error. And
> even if we got past that (by preventing the compiler from seeing the
> public declaration of the function when compiling it), it would invoke
> UB to declare a function differently from its definition, and then call
> that function.
since the function might be, and mostly is, compiled in a different translation
unit or sitting in a DSO here is where ABIs do matter.
> [1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html
> > Further I would like to add that Torvalds shares a similar view [2].
> >
> When has an argument from authority last worked for you? Does he have
> anything to say in that mail?
It was pointless to do that obviously, it was not my intent to diverse from what
I wrote/asked.
> [One read later]
> Well, if you look carefully, between the derision he actually managed to
> plug in the argument that glibc's open() implementation is (was) buggy,
> because it fails to support a use case devised nineteen years after the
> code was initially written.
> This seems to stem from the viewpoint that the open() libc function
> should be a thin wrapper around the SYS_open system call. And that is a
> fundamental misunderstanding. No, the libc's job is to provide an open()
> function that fulfills the contract set out in POSIX, and it might use
> the SYS_open system call to do it, or might not. That is an
> implementation detail.
> (Generalization of the above argument to _all_ system calls is left as
> an excercise to the reader).
> > Kind regards,
> > Norbert
> >
> > [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.
>
> Well, it'll make the window smaller. Also, if an application uses
> O_CLOEXEC, that means it expects FD_CLOEXEC to be set on the returned
> FD. Which has some detectable meaning, even if there is a window when
> the FD can leak to children of other threads.
> > diff -burN musl-1.1.21.org/src/fcntl/fcntl.c musl-1.1.21/src/fcntl/fcntl.c
> > --- musl-1.1.21.org/src/fcntl/fcntl.c 2019-04-10 13:35:43.589279396 +0200
> > +++ musl-1.1.21/src/fcntl/fcntl.c 2019-04-10 13:53:46.979931879 +0200
> > @@ -1,16 +1,12 @@
> > #define _GNU_SOURCE
> > +#define fcntl undef_fcntl
> > #include <fcntl.h>
> > -#include <stdarg.h>
> > #include <errno.h>
> > #include "syscall.h"
> > +#undef fcntl
> >
> My toenails are curling up at this...
Yeah, you could have taken from the example that I don't
want to touch declarations in header files.
Regards,
Norbert
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
