Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Wed, 8 Feb 2023 13:53:13 -0800
From: enh <enh@...gle.com>
To: Rich Felker <dalias@...c.org>
Cc: Khem Raj <raj.khem@...il.com>, musl@...ts.openwall.com
Subject: Re: SA_RESTORER for rv64?

yes, that looks like what we have in bionic for riscv64 (and what we
had for mips before it was removed). thanks!

On Wed, Feb 8, 2023 at 1:45 PM Rich Felker <dalias@...c.org> wrote:
>
> On Mon, Feb 06, 2023 at 12:49:53PM -0500, Rich Felker wrote:
> > On Mon, Feb 06, 2023 at 08:51:13AM -0800, enh wrote:
> > > On Sun, Feb 5, 2023 at 3:54 PM Rich Felker <dalias@...c.org> wrote:
> > > >
> > > > On Fri, Feb 03, 2023 at 10:44:56AM -0800, enh wrote:
> > > > > oops, never actually sent the patch. attached...
> > > > >
> > > > > On Thu, Nov 10, 2022 at 9:31 AM Khem Raj <raj.khem@...il.com> wrote:
> > > > > >
> > > > > > On Thu, Nov 10, 2022 at 9:19 AM Rich Felker <dalias@...c.org> wrote:
> > > > > > >
> > > > > > > On Thu, Nov 10, 2022 at 07:44:23AM -0800, enh wrote:
> > > > > > > > arch/riscv64/bits/signal.h has contained a definition for SA_RESTORER since
> > > > > > > > the initial commit, but i think that's just copy & paste from whichever
> > > > > > > > architecture the rv64 headers were based on? the linux kernel itself
> > > > > > > > doesn't have SA_RESTORER for rv64, unless i'm missing something?
> > > > > > >
> > > > > > > I suspect this is just a mistake. Have you seen any ill effects from
> > > > > > > it? If riscv folks can confirm it's wrong, I'll remove it.
> > > > > >
> > > > > > Yeah I think it should be removed. Perhaps mips is in same boat.
> > > > > >
> > > > > > >
> > > > > > > Rich
> > > >
> > > > > From 6413de6d9f785c98e5bc0cf40be947f1169d2fd7 Mon Sep 17 00:00:00 2001
> > > > > From: Elliott Hughes <enh@...gle.com>
> > > > > Date: Fri, 3 Feb 2023 10:42:55 -0800
> > > > > Subject: [PATCH] risc-v does not have SA_RESTORER.
> > > > >
> > > > > The kernel's include/uapi/asm-generic/signal-defs.h explicitly calls
> > > > > this out as obsolete. New architectures like risc-v do not define it.
> > > > > ---
> > > > >  arch/riscv64/bits/signal.h | 1 -
> > > > >  1 file changed, 1 deletion(-)
> > > > >
> > > > > diff --git a/arch/riscv64/bits/signal.h b/arch/riscv64/bits/signal.h
> > > > > index 287367db..fd6157a3 100644
> > > > > --- a/arch/riscv64/bits/signal.h
> > > > > +++ b/arch/riscv64/bits/signal.h
> > > > > @@ -76,7 +76,6 @@ typedef struct __ucontext
> > > > >  #define SA_RESTART   0x10000000
> > > > >  #define SA_NODEFER   0x40000000
> > > > >  #define SA_RESETHAND 0x80000000
> > > > > -#define SA_RESTORER  0x04000000
> > > > >
> > > > >  #endif
> > > > >
> > > > > --
> > > > > 2.39.1.519.gcb327c4b5f-goog
> > > > >
> > > >
> > > > I don't think this patch works as-is, since musl unconditionally uses
> > > > SA_RESTORER. We probably need to make that conditional on its
> > > > presence, and it looks like there's also a wrong-struct-layout issue
> > > > on archs where it's absent...
> > >
> > > yeah, bionic just uses the kernel uapi headers directly, and they look
> > > like this:
> > >
> > > struct sigaction {
> > >   __sighandler_t sa_handler;
> > >   unsigned long sa_flags;
> > > #ifdef SA_RESTORER
> > >   __sigrestore_t sa_restorer;
> > > #endif
> > >   sigset_t sa_mask;
> > > };
> >
> > OK. It looks like we need to remove the wrong SA_RESTORER for archs
> > that aren't supposed to have it *and* add such an #ifdef. Right now,
> > we're passing bogus sa_mask on these archs... :(
>
> How does the attached look?
>
> 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.