Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 18 Jan 2019 22:07:38 +0100
From: Arnd Bergmann <arnd@...db.de>
To: musl@...ts.openwall.com
Subject: Re: Re: Data structures defined by both linux and musl

On Fri, Jan 18, 2019 at 7:55 PM Rich Felker <dalias@...c.org> wrote:
> On Fri, Jan 18, 2019 at 06:06:01PM +0100, Arnd Bergmann wrote:
> > #if __BITS_PER_LONG == 64
> > #define LPSETTIMEOUT LPSETTIMEOUT_OLD
> > #else
> > #define LPSETTIMEOUT (sizeof(time_t) > sizeof(__kernel_long_t) ? \
> >     LPSETTIMEOUT_NEW : LPSETTIMEOUT_OLD)
> > #endif
> >
> > This way, we guarantee that we can still detect the data type
> > expected by an application calling LPSETTIMEOUT.
> > The same approach is used for setsockopt and some other
> > interfaces.
>
> Unless I'm misunderstanding something, this still leaves new programs
> using 64-bit time_t unable to make the ioctls on old kernels that lack
> the updated ioctl command. There's probably some significant subset of
> important commands that ioctl.c needs to be able to intercept and
> emulate.

That is correct. The number of ioctls that are affected here is
fairly small, and some of them are in rather obscure drivers,
but I was still hoping that we could avoid emulating them
in libc, and just require newer kernels for anyone who really
wants to run 64-bit time_t and use those drivers.

> > In other cases (in particular when we never pass absolute
> > CLOCK_REALTIME data), we changed the type inside
> > of a structure from time_t to 'long' or 'unsigned long', in
> > order to keep the ABI unchanged. The disadvantage here
> > is that it requires user space to use updated kernel headers,
> > which is a problem for applications that ship with a copy of
> > the kernel header.
>
> I think this is reasonable. It's not reasonable for kernel structures
> to have standard userspace types like time_t in them (except
> fixed-size ones like uint32_t, but kernel has __u32 for that anyway)
> and shipping copies of such headers was likewise a bug that should be
> corrected. It may be a moderate pain for distro ppl fixing this until
> the affected upstreams do, tho.

A lot of those structures have a long history, e.g. rusage has
always been defined in terms of timeval. I'd like to change this,
but it will be a lot of work to go through all uapi structures.

I can also understand the reasons for shipping copies of the
latest kernel headers in packages, this is not unlike what you
do in musl with its own version of the files. When a user space
package only cares about one header for a particular subsystem,
it tends to be easier to have a current copy of that header, and
just deal with binary compatibility for old kernels in the package,
than to have to support all combinations of library, header and
running kernel.

> > The same is true
> > for read/write based chardev interfaces such as /dev/input/eventX,
> > which we had to redefine to use a structure based on 'unsigned long'
>
> Uhg. How does this work with a 32-bit userspace running on a 64-bit
> kernel?! These should never have used long, only u32 or u64. Is it
> fixable? Or is there some reasonable way for userspace to detect which
> protocol the kernel is using?

We have a hack in that driver that detects when it's being called by
a compat mode thread, and another hack to detect x32 mode on
top of the first. Unfortunately, the kernel cannot detect the definition
of the time_t type that was used in the process reading the file
descriptor, so after long discussions we settled on leaving it at
the ABI and not adding more hacks to it, but requiring user space
to be aware that it has to use the updated kernel headers in this
case (which don't use time_t).

> > > > time_t may not be affected by these, but it shows that translation
> > > > is fragile in general, i wonder if we can ensure correct behaviour
> > > > in all cases. there is also the problem of linux headers which may
> > > > use and redefine libc types and user code may need to use those.
> > >
> > > Redefining libc types is already broken, and the kernel headers that
> > > do it can't be used from userspace when libc headers are included.
> > > This issue is independent of type sizes/layouts matching.
> > >
> > > I don't think any kernel headers _use_ libc types either. They
> > > generally use their own stuff.
> >
> > 'struct timespec' is a notable exception here, but probably not
> > the only one. At the moment, both libc and kernel define this
> > structure (and timeval, itimerval, itimerspec, ...), and in my
> > work on the kernel interfaces I assumed that the libc version
> > is the one that will prevail, while the kernel version should get
> > removed.
>
> Yes, I think any type defined by userspace standards/interface
> definitions inherently belongs to userspace implementation, and kernel
> headers should not touch it.

That is a sensible rule, it just doesn't match what has been done
historically. Now we have to retrofit it while trying to break as
little as possible in the process.

      Arnd

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.