linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Dmitry Vyukov <dvyukov@google.com>
To: aruna.ramakrishna@oracle.com, mathieu.desnoyers@efficios.com,
	 peterz@infradead.org, paulmck@kernel.org, boqun.feng@gmail.com
Cc: dave.hansen@linux.intel.com, jannh@google.com,
	jeffxu@chromium.org,  jorgelo@chromium.org,
	keescook@chromium.org, keith.lucas@oracle.com,
	 linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	mingo@kernel.org,  rick.p.edgecombe@intel.com,
	sroettger@google.com, tglx@linutronix.de,  x86@kernel.org
Subject: [PATCH v8 3/5] x86/pkeys: Update PKRU to enable all pkeys before XSAVE
Date: Tue,  4 Feb 2025 11:01:34 +0100	[thread overview]
Message-ID: <20250204100134.1843654-1-dvyukov@google.com> (raw)
In-Reply-To: <20240802061318.2140081-4-aruna.ramakrishna@oracle.com>

Re commit 70044df250d022572e26cd301bddf75eac1fe50e:
https://lore.kernel.org/all/20240802061318.2140081-4-aruna.ramakrishna@oracle.com/

> If the alternate signal stack is protected by a different pkey than the
> current execution stack, copying xsave data to the sigaltstack will fail
> if its pkey is not enabled in the PKRU register.
> 
> We do not know which pkey was used by the application for the altstack,
> so enable all pkeys before xsave.
> 
> But this updated PKRU value is also pushed onto the sigframe, which
> means the register value restored from sigcontext will be different from
> the user-defined one, which is unexpected. Fix that by overwriting the
> PKRU value on the sigframe with the original, user-defined PKRU.

Hi,

This unfortunatly seems to be broken for rseq user-space writes.
If the signal is caused by rseq struct being inaccessible due to PKEYs,
we try to write to rseq again at setup_rt_frame->rseq_signal_deliver,
which happens _before_ sig_prepare_pkru and won't succeed
(PKEY is still inaccessible, hard kills the process).
Any PKEY sandbox would want to restict untrusted access to rseq
as well (otherwise allows easy sandbox escapes).

If we do sig_prepare_pkru before rseq_signal_deliver (and generally
before any copy_to_userpace), then user-space handler gets SIGSEGV
and could unregister rseq and retry.

However, I am not sure if it's the best solution performance-
and complexity-wise (for user-space). A better solution may be to
change __rseq_handle_notify_resume to temporary switch to default
PKEY if user accesses fail.
Rseq is similar to signals in this respect. Since rseq updates
happen asynchronously with respect to user-space control flow,
if a program uses rseq and ever makes rseq inaccessible with PKEYs,
it's in trouble and will be randomly killed.
Since rseq updates are asynchronous as signals, they shouldn't
assume PKEY is set to default value that allows access
to rseq descriptor.

Thoughts?







  parent reply	other threads:[~2025-02-04 10:02 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-02  6:13 [PATCH v8 0/5] x86/pkeys: update " Aruna Ramakrishna
2024-08-02  6:13 ` [PATCH v8 1/5] x86/pkeys: Add PKRU as a parameter in signal handling functions Aruna Ramakrishna
2024-08-09 17:16   ` Jeff Xu
2024-08-02  6:13 ` [PATCH v8 2/5] x86/pkeys: Add helper functions to update PKRU on the sigframe Aruna Ramakrishna
2024-08-02  6:13 ` [PATCH v8 3/5] x86/pkeys: Update PKRU to enable all pkeys before XSAVE Aruna Ramakrishna
2024-08-09 17:30   ` Jeff Xu
2024-08-27  0:53     ` Jeff Xu
2024-10-03 23:29       ` Aruna Ramakrishna
2024-10-04  4:20         ` Jeff Xu
2025-02-04 10:01   ` Dmitry Vyukov [this message]
2025-02-06 18:06     ` Dmitry Vyukov
2025-02-10 22:46       ` Jeff Xu
2025-02-11  6:47         ` Dmitry Vyukov
2025-02-06 18:21     ` Dave Hansen
2025-02-06 18:35       ` Dmitry Vyukov
2024-08-02  6:13 ` [PATCH v8 4/5] x86/pkeys: Restore altstack access in sigreturn() Aruna Ramakrishna
2024-08-02  6:13 ` [PATCH v8 5/5] selftests/mm: Add new testcases for pkeys Aruna Ramakrishna
2024-08-02  8:16   ` Thomas Gleixner
2024-08-02  8:22   ` Thomas Gleixner
2024-08-02 12:04   ` Thomas Gleixner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250204100134.1843654-1-dvyukov@google.com \
    --to=dvyukov@google.com \
    --cc=aruna.ramakrishna@oracle.com \
    --cc=boqun.feng@gmail.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=jannh@google.com \
    --cc=jeffxu@chromium.org \
    --cc=jorgelo@chromium.org \
    --cc=keescook@chromium.org \
    --cc=keith.lucas@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mingo@kernel.org \
    --cc=paulmck@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rick.p.edgecombe@intel.com \
    --cc=sroettger@google.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox