linux-kernel - Re: [PATCH 2/2] sigaction.2: wfix

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <40c3c4cc-e135-1355-51ee-4d0f16e47e71@intel.com>
Date:   Mon, 8 Mar 2021 13:46:07 -0800
From:   "Yu, Yu-cheng" <yu-cheng.yu@...el.com>
To:     Borislav Petkov <bp@...en8.de>
Cc:     linux-man@...r.kernel.org,
        Alejandro Colomar <alx.manpages@...il.com>,
        Michael Kerrisk <mtk.manpages@...il.com>,
        Andy Lutomirski <luto@...nel.org>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Florian Weimer <fweimer@...hat.com>,
        "H.J. Lu" <hjl.tools@...il.com>, linux-kernel@...r.kernel.org,
        linux-api@...r.kernel.org
Subject: Re: [PATCH 2/2] sigaction.2: wfix - Clarify si_addr description.

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@...el.com>
>> Cc: Alejandro Colomar <alx.manpages@...il.com>
>> Cc: Michael Kerrisk <mtk.manpages@...il.com>
>> Cc: Andy Lutomirski <luto@...nel.org>
>> Cc: Borislav Petkov <bp@...en8.de>
>> Cc: Dave Hansen <dave.hansen@...ux.intel.com>
>> Cc: Florian Weimer <fweimer@...hat.com>
>> Cc: "H.J. Lu" <hjl.tools@...il.com>
>> Cc: linux-kernel@...r.kernel.org
>> Cc: linux-api@...r.kenel.org
>> Link: https://lore.kernel.org/linux-api/20210217222730.15819-7-yu-cheng.yu@intel.com/
>> ---
>>   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.
>