From: Dave Hansen <dave.hansen@intel.com>
To: Dmitry Vyukov <dvyukov@google.com>,
mathieu.desnoyers@efficios.com, peterz@infradead.org,
boqun.feng@gmail.com, tglx@linutronix.de, mingo@redhat.com,
bp@alien8.de, dave.hansen@linux.intel.com, hpa@zytor.com,
aruna.ramakrishna@oracle.com, elver@google.com
Cc: "Paul E. McKenney" <paulmck@kernel.org>,
x86@kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/4] x86/signal: Use switch_to_permissive_pkey_reg() helper
Date: Fri, 21 Feb 2025 08:26:46 -0800 [thread overview]
Message-ID: <d6d7745f-ad25-4faa-a3a3-65a4604e8592@intel.com> (raw)
In-Reply-To: <6a97e9986b0fb05935204f55f8d16fc5e66f0ccf.1739790300.git.dvyukov@google.com>
...
> -/*
> - * Enable all pkeys temporarily, so as to ensure that both the current
> - * execution stack as well as the alternate signal stack are writeable.
> - * The application can use any of the available pkeys to protect the
> - * alternate signal stack, and we don't know which one it is, so enable
> - * all. The PKRU register will be reset to init_pkru later in the flow,
> - * in fpu__clear_user_states(), and it is the application's responsibility
> - * to enable the appropriate pkey as the first step in the signal handler
> - * so that the handler does not segfault.
> - */
> -static inline u32 sig_prepare_pkru(void)
> -{
> - u32 orig_pkru = read_pkru();
> -
> - write_pkru(0);
> - return orig_pkru;
> -}
> -
> /*
> * Set up a signal frame.
> */
> @@ -157,8 +140,18 @@ get_sigframe(struct ksignal *ksig, struct pt_regs *regs, size_t frame_size,
> return (void __user *)-1L;
> }
>
> - /* Update PKRU to enable access to the alternate signal stack. */
> - pkru = sig_prepare_pkru();
> + /*
> + * Enable all pkeys temporarily, so as to ensure that both the current
> + * execution stack as well as the alternate signal stack are
> + * writeable. The application can use any of the available pkeys to
> + * protect the alternate signal stack, and we don't know which one it
> + * is, so enable all. The PKRU register will be reset to init_pkru
> + * later in the flow, in fpu__clear_user_states(), and it is the
> + * application's responsibility to enable the appropriate pkey as the
> + * first step in the signal handler so that the handler does not
> + * segfault.
> + */
> + pkru = switch_to_permissive_pkey_reg();
I think this hurts readability too much in the get_sigframe() code. On
some level, it's silly to have a basically empty helper. But in this
case, it does help keep the signal code readable.
In other words, this would be preferred:
/*
* Keep existing big comment
*/
static inline u32 sig_prepare_pkru(void)
{
return switch_to_permissive_pkey_reg();
}
next prev parent reply other threads:[~2025-02-21 16:26 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <cover.1739790300.git.dvyukov@google.com>
2025-02-17 11:07 ` [PATCH 1/4] pkeys: add API to switch to permissive pkey register Dmitry Vyukov
2025-02-17 20:03 ` Mathieu Desnoyers
2025-02-17 20:08 ` Mathieu Desnoyers
2025-02-17 20:09 ` Mathieu Desnoyers
2025-02-21 17:01 ` Dave Hansen
2025-02-24 13:25 ` Dmitry Vyukov
2025-02-25 16:15 ` Dave Hansen
2025-02-25 21:56 ` Dmitry Vyukov
2025-02-26 10:00 ` Dmitry Vyukov
2025-02-26 17:21 ` Dave Hansen
2025-02-27 13:58 ` Dmitry Vyukov
2025-02-21 17:37 ` Dave Hansen
2025-02-17 11:07 ` [PATCH 2/4] x86/signal: Use switch_to_permissive_pkey_reg() helper Dmitry Vyukov
2025-02-21 16:26 ` Dave Hansen [this message]
2025-02-24 13:13 ` Dmitry Vyukov
2025-02-17 11:07 ` [PATCH 3/4] rseq: Make rseq work with protection keys Dmitry Vyukov
2025-02-17 20:21 ` Mathieu Desnoyers
2025-02-18 7:55 ` Dmitry Vyukov
2025-02-18 14:57 ` Mathieu Desnoyers
2025-02-18 15:10 ` Dmitry Vyukov
2025-02-18 15:27 ` Mathieu Desnoyers
2025-02-18 15:37 ` Dmitry Vyukov
2025-02-21 11:22 ` Dmitry Vyukov
2025-02-21 19:41 ` Mathieu Desnoyers
2025-02-21 17:17 ` Dave Hansen
2025-02-21 19:38 ` Mathieu Desnoyers
2025-02-21 19:48 ` Dave Hansen
2025-02-21 20:05 ` Mathieu Desnoyers
2025-02-21 20:50 ` Dave Hansen
2025-02-21 21:11 ` Mathieu Desnoyers
2025-02-21 21:36 ` Mathieu Desnoyers
2025-02-21 21:45 ` Dave Hansen
2025-02-24 13:35 ` Dmitry Vyukov
2025-02-21 21:40 ` Dave Hansen
2025-02-17 11:07 ` [PATCH 4/4] selftests/rseq: Add test for rseq+pkeys Dmitry Vyukov
2025-02-17 20:23 ` Mathieu Desnoyers
2025-02-21 17:24 ` Dave Hansen
2025-02-24 13:22 ` Dmitry Vyukov
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=d6d7745f-ad25-4faa-a3a3-65a4604e8592@intel.com \
--to=dave.hansen@intel.com \
--cc=aruna.ramakrishna@oracle.com \
--cc=boqun.feng@gmail.com \
--cc=bp@alien8.de \
--cc=dave.hansen@linux.intel.com \
--cc=dvyukov@google.com \
--cc=elver@google.com \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mathieu.desnoyers@efficios.com \
--cc=mingo@redhat.com \
--cc=paulmck@kernel.org \
--cc=peterz@infradead.org \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.