linux-kernel - Re: [PATCH 8/9] RISC-V: User-facing API

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <mhng-69cb8e74-bdd5-41b8-9955-748be5e45af4@palmer-si-x1c4>
Date:   Wed, 05 Jul 2017 09:49:36 -0700 (PDT)
From:   Palmer Dabbelt <palmer@...belt.com>
To:     james.hogan@...tec.com
CC:     albert@...ive.com
Subject:     Re: [PATCH 8/9] RISC-V: User-facing API

On Mon, 03 Jul 2017 16:06:39 PDT (-0700), james.hogan@...tec.com wrote:
> On Thu, Jun 29, 2017 at 02:42:38PM -0700, Palmer Dabbelt wrote:
>> On Wed, 28 Jun 2017 15:42:37 PDT (-0700), james.hogan@...tec.com wrote:
>> > On Wed, Jun 28, 2017 at 11:55:37AM -0700, Palmer Dabbelt wrote:
>> >> diff --git a/arch/riscv/include/uapi/asm/ucontext.h b/arch/riscv/include/uapi/asm/ucontext.h
>> >> new file mode 100644
>> >> index 000000000000..52eff9febcfd
>> >> --- /dev/null
>> >> +++ b/arch/riscv/include/uapi/asm/ucontext.h
>> > ...
>> >> +struct ucontext {
>> >> +	unsigned long	  uc_flags;
>> >> +	struct ucontext	 *uc_link;
>> >> +	stack_t		  uc_stack;
>> >> +	sigset_t	  uc_sigmask;
>> >> +	/* glibc uses a 1024-bit sigset_t */
>> >> +	__u8		  __unused[1024 / 8 - sizeof(sigset_t)];
>> >> +	/* last for future expansion */
>> >> +	struct sigcontext uc_mcontext;
>> >> +};
>> >
>> > Any particular reason not to use the asm-generic ucontext?
>>
>> In the generic ucontext, 'uc_sigmask' is at the end of the structure so it can
>> be expanded.  Since we want our mcontext to be expandable as well, we
>> pre-allocate some expandable space for sigmask and then put mcontext at the
>> end.
>>
>> We stole this idea from arm64.
>
> Curious. __unused seems like overkill to be honest given that expanding
> the number of signals up to 128 causes other issues (as discovered on
> MIPS e.g. the waitpid() status, with stopsig not fitting below the exit
> code (shift 8) and core dump flag (bit 7)), but perhaps it could be
> carefully expanded by splitting the stopsig field.

Sorry, I don't understand the intricacies of this in the slightest.  In general
we try to avoid surprises in software land in RISC-V, so whenever we do
something we go look at the most popular architectures (Intel and ARM) and try
to ensure we don't paint ourselves into any corners that they didn't.

> Looks harmless here I suppose so I defer to others. If it is the
> preferred approach does it make sense to make it the "default" for new
> architectures at some point?

Again, this isn't really my thing, but we chose this because we thought it was
the sane way to do it.  Unless we're doing something silly, I don't see why it
wouldn't be a reasonable default.  This is predicated on having expandable
architectural state, otherwise putting sigmask at the end seems sane.