Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Thu, 11 Apr 2019 12:23:26 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: Re: use of varargs in open and various functions

On Thu, Apr 11, 2019 at 06:03:16PM +0200, Norbert Lange wrote:
> >
> > 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.

Indeed, that detail is not relevant to your specific changes, but the
point was to highlight that there *are* ABI subtleties here and doing
it wrong for the sake of liking the wrong way of doing it is asking
for trouble.

> > 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.

That's not how it works. Symbolic tools do not have "disable LTO for
these functions". See also things like wasm.

> > > 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).

Basically nobody uses syscall(), except some very low-level software
or software using hacks to get access to bleeding-edge functionality.
It's a junk interface that shouldn't exist publicly. If it can't be
supported in some environments, that's largely okay. On the other hand
everything uses open.

> > 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.

It has to be that way because that's the public interface definition,
and the language does not make the two forms equivalent.

> > > [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.
> 
> To me that sounds like some min required kernel version, would make
> a useful configure option.

We don't have configure options like that, and don't have any hard min
kernel version. Back to 2.5.x "fully works" with the functionality
limitations (lack of *at functions, etc.) and bugs (especially lots of
signal-related bugs) those versions imply. Back to 2.4.0 or earlier
works on some archs, without threads (pthread_create will fail). The
buggy versions are somewhere in the late 2.6.x or 3.x range IIRC, way
above the "fully works" threshold. So the question is whether they
were every widely distributed without being fixed in environments
someone might encounter in the wild where fd leakage would matter.

Rich

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.