| 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
| ||
|
Date: Wed, 13 Sep 2017 10:12:49 -0700 (PDT)
From: Palmer Dabbelt <palmer@...belt.com>
To: Arnd Bergmann <arnd@...db.de>
CC: peterz@...radead.org, tglx@...utronix.de, jason@...edaemon.net,
marc.zyngier@....com, dmitriy@...-tech.org,
yamada.masahiro@...ionext.com, mmarek@...e.com, albert@...ive.com,
will.deacon@....com, boqun.feng@...il.com, oleg@...hat.com,
mingo@...hat.com, daniel.lezcano@...aro.org,
gregkh@...uxfoundation.org, jslaby@...e.com, davem@...emloft.net,
mchehab@...nel.org, hverkuil@...all.nl, rdunlap@...radead.org,
viro@...iv.linux.org.uk, mhiramat@...nel.org, fweisbec@...il.com,
mcgrof@...nel.org, dledford@...hat.com, bart.vanassche@...disk.com,
sstabellini@...nel.org, mpe@...erman.id.au,
rmk+kernel@...linux.org.uk, paul.gortmaker@...driver.com,
nicolas.dichtel@...nd.com, linux@...ck-us.net,
heiko.carstens@...ibm.com, schwidefsky@...ibm.com,
geert@...ux-m68k.org, akpm@...ux-foundation.org,
andriy.shevchenko@...ux.intel.com, jiri@...lanox.com,
vgupta@...opsys.com, airlied@...hat.com, jk@...abs.org,
chris@...is-wilson.co.uk, Jason@...c4.com,
paulmck@...ux.vnet.ibm.com, ncardwell@...gle.com,
linux-kernel@...r.kernel.org, linux-kbuild@...r.kernel.org,
patches@...ups.riscv.org
Subject: Re: [PATCH v8 17/18] RISC-V: User-facing API
On Wed, 13 Sep 2017 08:56:54 PDT (-0700), Arnd Bergmann wrote:
> On Tue, Sep 12, 2017 at 11:57 PM, Palmer Dabbelt <palmer@...belt.com> wrote:
>> This patch contains code that is in some way visible to the user:
>> including via system calls, the VDSO, module loading and signal
>> handling. It also contains some generic code that is ABI visible.
>
> I took a good look at this and found nothing that is really wrong, but
> I noticed two smaller issues that I'd like to bring up for discussion:
>
>> 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
>> @@ -0,0 +1,35 @@
>> + * This file was copied from arch/arm64/include/uapi/asm/ucontext.h
>> + */
>> +#ifndef _UAPI__ASM_UCONTEXT_H
>> +#define _UAPI__ASM_UCONTEXT_H
>> +
>> +#include <linux/types.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;
>> +};
>
> This seems odd, the arm64 file was added with this comment
>
> commit 33b36543df336d9158e1a763fe97251885f52c5c
> Author: Will Deacon <will.deacon@....com>
> Date: Fri Jan 16 13:52:14 2015 +0000
>
> arm64: uapi: expose our struct ucontext to the uapi headers
>
> arm64 defines its own ucontext structure which is incompatible with the
> struct defined (and exposed to userspace by) the asm-generic headers.
>
> glibc carries its own struct definition that is compatible with the
> arm64 definition, but we should expose our format in the uapi headers in
> case other libraries want to make use of the ucontext pushed as part of
> an arm64 sigframe.
>
> This patch moves the arm64 asm/ucontext.h to the uapi headers, along
> with the necessary #include of linux/types.h.
>
> which doesn't really explain _why_ they are different from asm-generic.
>
> Can you explain this? Does the ARM64 layout have a significant
> advantage over the asm-generic one, or is it just what you happened
> to use because you copied from ARM64?
I copied those comments from ARM64, and they're pretty useless. How does this
look?
diff --git a/arch/riscv/include/uapi/asm/ucontext.h b/arch/riscv/include/uapi/asm/ucontext.h
index 52eff9febcfd..1fae8b1697e0 100644
--- a/arch/riscv/include/uapi/asm/ucontext.h
+++ b/arch/riscv/include/uapi/asm/ucontext.h
@@ -26,9 +26,19 @@ struct ucontext {
struct ucontext *uc_link;
stack_t uc_stack;
sigset_t uc_sigmask;
- /* glibc uses a 1024-bit sigset_t */
+ /* There's some padding here to allow sigset_t to be expanded in the
+ * future. Though this is unlikely, other architectures put uc_sigmask
+ * at the end of this structure and explicitly state it can be
+ * expanded, so we didn't want to box ourselves in here. */
__u8 __unused[1024 / 8 - sizeof(sigset_t)];
- /* last for future expansion */
+ /* We can't put uc_sigmask at the end of this structure because we need
+ * to be able to expand sigcontext in the future. For example, the
+ * vector ISA extension will almost certainly add ISA state. We want
+ * to ensure all user-visible ISA state can be saved and restored via a
+ * ucontext, so we're putting this at the end in order to allow for
+ * infinite extensibility. Since we know this will be extended and we
+ * assume sigset_t won't be extended an extreme amount, we're
+ * prioritizing this. */
struct sigcontext uc_mcontext;
};
> If that layout is indeed better, maybe we should change asm-generic
> to use that, and fall back to the old layout for the architectures that
> already use it.
That cropped up during glibc as well, and I think it might be the right answer.
Should I submit another patch that fixes up the other ISAs?
> If one is as good as the other, could you change kernel and glibc
> to use the normal one instead?
>
>> + */
>> +VERSION
>> +{
>> + LINUX_2.6 {
>> + global:
>> + __vdso_rt_sigreturn;
>> + __vdso_cmpxchg32;
>> + __vdso_cmpxchg64;
>> + local: *;
>> + };
>
> The last vdso that got added was for arm64, and it was still
> during linux-2.6 times.
>
> Should this instead use the version that you are targetting for
> the merge, i.e. 4.15?
That sounds right to me
diff --git a/arch/riscv/kernel/vdso/vdso.lds.S b/arch/riscv/kernel/vdso/vdso.lds.S
index 7142e1aafc30..8c9dce95c11d 100644
--- a/arch/riscv/kernel/vdso/vdso.lds.S
+++ b/arch/riscv/kernel/vdso/vdso.lds.S
@@ -67,7 +67,7 @@ PHDRS
*/
VERSION
{
- LINUX_2.6 {
+ LINUX_4.15 {
global:
__vdso_rt_sigreturn;
__vdso_cmpxchg32;
Powered by blists - more mailing lists