Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Fri, 2 Jun 2023 11:30:05 -0400
From: Rich Felker <dalias@...c.org>
To: Markus Wichmann <nullplan@....net>
Cc: musl@...ts.openwall.com
Subject: Re: [PATCH] [RFC] make clone() usable

On Fri, Jun 02, 2023 at 04:19:47PM +0200, Markus Wichmann wrote:
> Am Thu, Jun 01, 2023 at 04:08:07PM -0400 schrieb Rich Felker:
> > On Thu, Jun 01, 2023 at 01:12:57PM +0300, Alexey Izbyshev wrote:
> > > Another thing that might be somewhat relevant if we expect to see
> > > wider usage of clone is that syscall (the public function) currently
> > > blindly extracts six arguments via va_arg, which on some
> > > architectures requires sufficient stack space not to crash. So on
> > > i386 the following silly function will crash if passed to clone,
> > > provided that "stack" points to the beginning of unmapped space past
> > > the stack page end.
> >
> > That's changed in the patch. It only calls va_arg for arguments whose
> > existence the flags implies.
> >
> > Rich
> 
> clone() does, yes, but syscall() doesn't. So if the function handed to
> clone() calls the syscall() function at the top of the stack, it crashes
> on architectures that pass parameters on stack primarily.
> 
> Quick fix would be to make __clone() on those architectures reserve
> sufficient stack space for six parameters before calling the client
> function. E.g. on i386, one parameter is four bytes, so six parameters
> are twenty-four bytes, and I believe the stack alignment was sixteen
> bytes, right? So __clone() could just reduce %esp by thirty-two before
> calling the function pointer.

It's a hack that might be worthwhile.

> The proper fix would be to have syscall() only read as many arguments as
> were given. Since syscall() is an extension function, nobody can rely on
> it being defined as

That requires knowing all current and future syscalls... :(

> long syscall(int, ...);
> 
> You might use similar macros to the ones in syscall.h to add another
> argument for the number of arguments. That would avoid the crash.

Yes, that's perhaps a good idea. Though it does potentially make a
mess with how macros affect namespace...

> Of course, you have to keep the above function around for ABI reasons.
> Nothing's ever easy, right?

Yes, though it's no worse than now if existing binaries that were
crashing still crash...

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.