Follow @Openwall on Twitter for new release announcements and other news
[<prev] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20261104131903.GS23438@brightrain.aerifal.cx>
Date: Wed, 4 Nov 2026 08:19:03 -0500
From: Rich Felker <dalias@...c.org>
To: Imre Kaloz <kaloz@...nel.org>
Cc: musl@...ts.openwall.com
Subject: Re: [PATCH 04/10] add SYSCALL_RT_SIGACTION_RESTORER hook for
 sparc

On Wed, Nov 04, 2026 at 08:15:01AM -0500, Rich Felker wrote:
> On Wed, Nov 04, 2026 at 10:12:03AM +0100, Imre Kaloz wrote:
> > Hi Rich,
> > 
> > On Wed, Nov 4, 2026, at 04:44, Rich Felker wrote:
> > 
> > > I think this patch misses other places SYS_rt_sigaction is used, and
> > > puts arch-specific ifdefs we shouldn't be doing inline each place the
> > > syscall is used.
> > 
> > Right, abort() still makes the generic call, which on sparc passes
> > _NSIG/8 as the restorer and no valid sigsetsize, so resetting SIGABRT
> > to SIG_DFL fails.
> > 
> > > Probably we should either (or both) replace direct use of the syscall
> > > with a __sys_sigaction (like __sys_open, etc.) defined in
> > > src/internal/syscall.h if the arch doesn't first define it, or #undef
> > > SYS_rt_sigaction on sparc and have a case for where that's not
> > > defined, to be safe against missing instances of its use or adding new
> > > ones with silent breakage.
> > 
> > I'd do both. The generic part is below as an RFC; with it, sparc64
> > would define
> > 
> > #define __sys_sigaction(sig,sa,old) \
> > 	__syscall(__NR_rt_sigaction,sig,sa,old,__restore,_NSIG/8)
> > #undef SYS_rt_sigaction
> 
> I'm pretty sure this breaks expansion of __sys_sigaction.
> 
> > > Note that since __restore and __restore_rt are the same,
> > > __sys_sigaction does not need a conditional on flags. It can just
> > > always pass __restore in the extra slot.
> > 
> > They are on sparc64. 32-bit sparc builds a different frame for
> > handlers without SA_SIGINFO and returns from it with sigreturn rather
> > than rt_sigreturn, so there the restorer has to follow SA_SIGINFO:
> > 
> > #define __sys_sigaction(sig,sa,old) \
> > 	__syscall(__NR_rt_sigaction,sig,sa,old, \
> > 		(long)(sa) && ((sa)->flags & SA_SIGINFO) ? __restore_rt : __restore, \
> > 		_NSIG/8)
> > #undef SYS_rt_sigaction
> > 
> > (sa is tested as an integer because gcc warns on the address of
> > abort's compound literal.) Does this look right before I fold it
> > into v2?
> 
> Which warning is this? I don't like casts to avoid warnings.
> 
> It might be better to write an inline function by the same name and
> just define the macro to itself. That gets rid of the multi-line
> macros too.

The inline function would also fix the issue with undefining the
SYS_rt_sigaction macro, but I think it makes a new issue by depending
on _NSIG. Maybe the macro can add _NSIG in the arguments to the inline
function.

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.