Re: [PATCH 2/2] sigaction.2: wfix - Clarify si_addr description.

[Yu, Yu-cheng]

On 3/8/2021 1:30 PM, Borislav Petkov wrote:
> On Fri, Feb 26, 2021 at 09:26:34AM -0800, Yu-cheng Yu wrote:
> > SIGSEGV fills si_addr only for memory access faults.  Add a note to clarify.
> >
> > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@xxxxxxxxx>
> > Cc: Alejandro Colomar <alx.manpages@xxxxxxxxx>
> > Cc: Michael Kerrisk <mtk.manpages@xxxxxxxxx>
> > Cc: Andy Lutomirski <luto@xxxxxxxxxx>
> > Cc: Borislav Petkov <bp@xxxxxxxxx>
> > Cc: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
> > Cc: Florian Weimer <fweimer@xxxxxxxxxx>
> > Cc: "H.J. Lu" <hjl.tools@xxxxxxxxx>
> > Cc: linux-kernel@xxxxxxxxxxxxxxx
> > Cc: linux-api@xxxxxxxxxxxxxx
> > Link: https://lore.kernel.org/linux-api/20210217222730.15819-7-yu-cheng.yu@xxxxxxxxx/
> > ---
> >   man2/sigaction.2 | 7 ++++++-
> >   1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/man2/sigaction.2 b/man2/sigaction.2
> > index 49a30f11e..bea884a23 100644
> > --- a/man2/sigaction.2
> > +++ b/man2/sigaction.2
> > @@ -467,7 +467,7 @@ and
> >   .BR SIGTRAP
> >   fill in
> >   .I si_addr
> > -with the address of the fault.
> > +with the address of the fault (see notes).
> >   On some architectures,
> >   these signals also fill in the
> >   .I si_trapno
> > @@ -955,6 +955,11 @@ It is not possible to block
> >   .IR sa_mask ).
> >   Attempts to do so are silently ignored.
> >   .PP
> > +In a
> > +.B SIGSEGV,
> > +if the fault is a memory access fault, si_addr is filled with the address
> > +causing the fault, otherwise it is not filled.
>
> "... otherwise it is uninitialized." or "zeroed" or whatever...
>
> And I'm having trouble figuring out why do you need to clarify this?
>
> Because of this sentence:
>
>         * SIGILL,  SIGFPE, SIGSEGV, SIGBUS, and SIGTRAP fill in si_addr with the address
>           of the fault.  On some architectures, these signals also fill in the si_trapno
>           field.
>
> ?

I think the sentence above is vague, but probably for the reason that 
each arch is different.  Maybe this patch is unnecessary and can be dropped?

> If so, did you audit all architectures whether si_addr is populated only
> on memory access faults or is this something POSIX dictates or what's
> up? Because the sigaction(2) manpage is arch-agnostic and this is a
> rather strong assertion.
>
> What am I missing?
>
> Thx.