[PATCH RFC v3 2/9] virtio_net: Add functions for hashing
+
+static inline void virtio_net_toeplitz(struct virtio_net_toeplitz_state *state,
+ const __be32 *input, size_t len)
The function calculates a hash value but its name does not make it
clear. Consider adding a 'calc'.
+{
+ u32 key;
+
+ while (len) {
+ state->key++;
+ key = be32_to_cpu(*state->key);
You perform be32_to_cpu to support both CPU endianities.
If you will follow with an unconditional swab32, you could run the
following loop on a more natural 0 to 31 always referring to bit 0
and avoiding !!(key & bit):
key = swab32(be32_to_cpu(*state->key));
for (i = 0; i < 32; i++, key >>= 1) {
if (be32_to_cpu(*input) & 1)
state->hash ^= state->key_buffer;
state->key_buffer = (state->key_buffer << 1) | (key & 1);
}
+
+ for (u32 bit = BIT(31); bit; bit >>= 1) {
+ if (be32_to_cpu(*input) & bit)
+ state->hash ^= state->key_buffer;
+
+ state->key_buffer =
+ (state->key_buffer << 1) | !!(key & bit);
+ }
+
+ input++;
+ len--;
+ }
+}
+
+static inline u32 virtio_net_hash_report(u32 types,
+struct flow_dissector_key_basic key)
+{
+ switch (key.n_proto) {
+ case htons(ETH_P_IP):
Other parts of the code use be_to_cpu and cpu_to_be, Why use legacy
htons here?
+ if (key.ip_proto == IPPROTO_TCP &&
+ (types & VIRTIO_NET_RSS_HASH_TYPE_TCPv4))
+ return VIRTIO_NET_HASH_REPORT_TCPv4;
+
+ if (key.ip_proto == IPPROTO_UDP &&
+ (types & VIRTIO_NET_RSS_HASH_TYPE_UDPv4))
+ return VIRTIO_NET_HASH_REPORT_UDPv4;
+
+ if (types & VIRTIO_NET_RSS_HASH_TYPE_IPv4)
+ return VIRTIO_NET_HASH_REPORT_IPv4;
+
+ return VIRTIO_NET_HASH_REPORT_NONE;
+
+ case htons(ETH_P_IPV6):
+ if (key.ip_proto == IPPROTO_TCP &&
+ (types & VIRTIO_NET_RSS_HASH_TYPE_TCPv6))
+ return VIRTIO_NET_HASH_REPORT_TCPv6;
+
+ if (key.ip_proto == IPPROTO_UDP &&
+ (types & VIRTIO_NET_RSS_HASH_TYPE_UDPv6))
+ return VIRTIO_NET_HASH_REPORT_UDPv6;
+
+ if (types & VIRTIO_NET_RSS_HASH_TYPE_IPv6)
+ return VIRTIO_NET_HASH_REPORT_IPv6;
+
+ return VIRTIO_NET_HASH_REPORT_NONE;
+
+ default:
+ return VIRTIO_NET_HASH_REPORT_NONE;
+ }
+}
#endif /* _LINUX_VIRTIO_NET_H */
--
2.46.0
[PATCH RFC v3 2/9] virtio_net: Add functions for hashing
> +
> +static inline void virtio_net_toeplitz(struct virtio_net_toeplitz_state
> *state,
> +const __be32 *input, size_t len)
>
> The function calculates a hash value but its name does not make it
> clear. Consider adding a 'calc'.
>
> +{
> + u32 key;
> +
> + while (len) {
> + state->key++;
> + key = be32_to_cpu(*state->key);
>
> You perform be32_to_cpu to support both CPU endianities.
> If you will follow with an unconditional swab32, you could run the
> following loop on a more natural 0 to 31 always referring to bit 0
> and avoiding !!(key & bit):
>
> key = swab32(be32_to_cpu(*state->key));
> for (i = 0; i < 32; i++, key >>= 1) {
> if (be32_to_cpu(*input) & 1)
> state->hash ^= state->key_buffer;
> state->key_buffer = (state->key_buffer << 1) | (key & 1);
> }
>
Fixing myself, in previous version 'input' was tested against same bit.
Advantage is less clear now, replacing !! with extra shift.
However, since little endian CPUs are more common, the combination of
swab32(be32_to_cpu(x) will actually become a nop.
Similar tactic may be applied to 'input' by assigning it to local
variable. This may produce more efficient version but not necessary
easier to understand.
key = bswap32(be32_to_cpu(*state->key));
for (u32 bit = BIT(31); bit; bit >>= 1, key >>= 1) {
if (be32_to_cpu(*input) & bit)
state->hash ^= state->key_buffer;
state->key_buffer =
(state->key_buffer << 1) | (key & 1);
}
>
> +
> + for (u32 bit = BIT(31); bit; bit >>= 1) {
> + if (be32_to_cpu(*input) & bit)
> + state->hash ^= state->key_buffer;
> +
> + state->key_buffer =
> + (state->key_buffer << 1) | !!(key & bit);
> + }
> +
> + input++;
> + len--;
> + }
> +}
> +
Re: [PATCH] kernel-docs: Add new section for Rust learning materials
On Wed, Sep 11, 2024 at 8:59 PM Carlos Bilbao wrote: > > Include a new section in the Index of Further Kernel Documentation with > resources to learn Rust. Reference it in the Rust index. Thanks for this, Carlos! It would be nice to mention that this came out of a session of Kangrejos with suggestions from people in the room (or who suggested it). Did you rank/filter them in some way? i.e. my plan was to perhaps make a poll or something in Zulip and then pick the best ones. A few extra that got mentioned: https://rust-book.cs.brown.edu (perhaps could go into the Rust book entry somehow; having said that, I am not sure if it is being updated, and it is part of an "experiment"), https://newrustacean.com, the reference, https://github.com/rust-lang/opsem-team/blob/main/fcps.md... > +* Name: **Linux Plumbers (LPC) Rust presentations** I wonder if listing individual talks may be a bit too much, compared to other entries in the file that link to the overall resource. Perhaps LPC should be in a different section as a "global" resource, perhaps with links to the few latest years' full schedules? > +* Name: **The Rustacean Station Podcast** By the way, are these sorted in any particular way? Cheers, Miguel
[PATCH RFC v3 2/9] virtio_net: Add functions for hashing
> +
> +static inline bool virtio_net_hash_rss(const struct sk_buff *skb,
> +u32 types, const __be32 *key,
> +struct virtio_net_hash *hash)
Based on the guidelines, this function seems imperative rather than
predicate and should return an error-code integer.
https://www.kernel.org/doc/html/latest/process/coding-style.html#function-return-values-and-names
> +{
> + u16 report;
> + struct virtio_net_toeplitz_state toeplitz_state = {
> + .key_buffer = be32_to_cpu(*key),
> + .key = key
> + };
> + struct flow_keys flow;
> +
> + if (!skb_flow_dissect_flow_keys(skb, &flow, 0))
> + return false;
> +
> + report = virtio_net_hash_report(types, flow.basic);
> +
> + switch (report) {
> + case VIRTIO_NET_HASH_REPORT_IPv4:
> + virtio_net_toeplitz(&toeplitz_state,
> + (__be32 *)&flow.addrs.v4addrs,
> + sizeof(flow.addrs.v4addrs) / 4);
> + break;
> +
> + case VIRTIO_NET_HASH_REPORT_TCPv4:
> + virtio_net_toeplitz(&toeplitz_state,
> + (__be32 *)&flow.addrs.v4addrs,
> + sizeof(flow.addrs.v4addrs) / 4);
> + virtio_net_toeplitz(&toeplitz_state, &flow.ports.ports,
> + 1);
> + break;
> +
> + case VIRTIO_NET_HASH_REPORT_UDPv4:
> + virtio_net_toeplitz(&toeplitz_state,
> + (__be32 *)&flow.addrs.v4addrs,
> + sizeof(flow.addrs.v4addrs) / 4);
> + virtio_net_toeplitz(&toeplitz_state, &flow.ports.ports,
> + 1);
> + break;
> +
> + case VIRTIO_NET_HASH_REPORT_IPv6:
> + virtio_net_toeplitz(&toeplitz_state,
> + (__be32 *)&flow.addrs.v6addrs,
> + sizeof(flow.addrs.v6addrs) / 4);
> + break;
> +
> + case VIRTIO_NET_HASH_REPORT_TCPv6:
> + virtio_net_toeplitz(&toeplitz_state,
> + (__be32 *)&flow.addrs.v6addrs,
> + sizeof(flow.addrs.v6addrs) / 4);
> + virtio_net_toeplitz(&toeplitz_state, &flow.ports.ports,
> + 1);
> + break;
> +
> + case VIRTIO_NET_HASH_REPORT_UDPv6:
> + virtio_net_toeplitz(&toeplitz_state,
> + (__be32 *)&flow.addrs.v6addrs,
> + sizeof(flow.addrs.v6addrs) / 4);
> + virtio_net_toeplitz(&toeplitz_state, &flow.ports.ports,
> + 1);
> + break;
> +
> + default:
> + return false;
> + }
> +
> + hash->value = toeplitz_state.hash;
> + hash->report = report;
> +
> + return true;
> +}
> +
Re: [PATCH v5 00/14] integrity: Introduce the Integrity Digest Cache
On Thu, Sep 05, 2024 at 05:05:29PM +0200, Roberto Sassu wrote: Good morning, I hope the week is starting well for everyone Apologies for the delay in getting these thoughts out, scrambling to catch up on my e-mail backlog. I looped Linus in, secondary to the conversations surrounding the PGP verification infrastructure in the kernel, given that the primary use case at this time appears to be the digest cache and his concerns regarding that use. Our proposed TSEM LSM, most recent submission here: https://lore.kernel.org/linux-security-module/20240826103728.3378-1-g...@enjellic.com/T/#t Is a superset of IMA functionality and depends heavily on file checksums, hence our interest and reflections in your efforts with this. > From: Roberto Sassu > > Integrity detection and protection has long been a desirable feature, to > reach a large user base and mitigate the risk of flaws in the software > and attacks. > > However, while solutions exist, they struggle to reach a large user base, > due to requiring higher than desired constraints on performance, > flexibility and configurability, that only security conscious people are > willing to accept. No argument here, inherent in better and more effective security architectures is better useability, pure and simple. > For example, IMA measurement requires the target platform to collect > integrity measurements, and to protect them with the TPM, which > introduces a noticeable overhead (up to 10x slower in a > microbenchmark) on frequently used system calls, like the open(). The future for trusted systems will not be in TPM's, as unpopular a notion as that may be in some circles. They represent a design from a quarter century ago that struggles to have relevance with our current system architectures. If a TPM is present, TSEM will extend the security coefficients for the root modeling namespace into a PCR to establish a root of trust that the rest of the trust orchestration system can be built on. Ours is a worst case scenario beyond IMA since there is a coefficient generated for each LSM call that is being modeled. We had to go to asynchronous updates through an ordered workqueue in order to have something less than abysmal performance, even with vTPM's running in a Xen hypervisor domain. This is without the current performance impacts being discussed with respect to HMAC based TPM session authentication. > IMA Appraisal currently requires individual files to be signed and > verified, and Linux distributions to rebuild all packages to include > file signatures (this approach has been adopted from Fedora > 39+). Like a TPM, also signature verification introduces a > significant overhead, especially if it is used to check the > integrity of many files. > > This is where the new Integrity Digest Cache comes into play, it > offers additional support for new and existing integrity solutions, > to make them faster and easier to deploy. > > The Integrity Digest Cache can help IMA to reduce the number of TPM > operations and to make them happen in a deterministic way. If IMA > knows that a file comes from a Linux distribution, it can measure > files in a different way: measure the list of digests coming from > the distribution (e.g. RPM package headers), and subsequently > measure a file if it is not found in that list. > > The performance improvement comes at the cost of IMA not reporting > which files from installed packages were accessed, and in which > temporal sequence. This approach might not be suitable for all use > cases. That, in and of itself, is certainly not the end of the world. With TSEM we offer the notion of the 'state' of a security namespace, which is the extension sum of the security coefficients after they have been sorted in natural (big-endian) hash order. In this model you know what files have been accessed but you do not have a statement on temporal ordering of access. Given scheduling artifacts, let alone the almost absolute ubiquity of multi-core, the simple TPM/TCG linear extension model seems to struggle with respect to any relevancy as a security metric. > The Integrity Digest Cache can also help IMA for appraisal. IMA can simply > lookup the calculated digest of an accessed file in the list of digests > extracted from package headers, after verifying the header signature. It is > sufficient to verify only one signature for all files in the package, as > opposed to verifying a signature for each file. > > The same approach can be followed by other LSMs, such as Integrity Policy > Enforcement (IPE), and BPF LSM. As we've noted above, TSEM would also be a potential consumer, which is why we wanted to seek clarifications on the architecture. We've reviewed the patch set and the documentation, and will freely admit that we may still misunderstand all of this, but it would seem that the architecture, as it stands, would be subject to Time Of Measurement Time Of Use (TOMTOU) challenges. The Time Of Measurement will be when th
Re: [PATCH v10 14/14] riscv: Add ghostwrite vulnerability
On Wed, Sep 11, 2024 at 10:55:22PM -0700, Charlie Jenkins wrote:
> Follow the patterns of the other architectures that use
> GENERIC_CPU_VULNERABILITIES for riscv to introduce the ghostwrite
> vulnerability and mitigation. The mitigation is to disable all vector
> which is accomplished by clearing the bit from the cpufeature field.
>
> Ghostwrite only affects thead c9xx CPUs that impelment xtheadvector, so
> the vulerability will only be mitigated on these CPUs.
>
> Signed-off-by: Charlie Jenkins
> ---
> arch/riscv/Kconfig.errata| 11
> arch/riscv/errata/thead/errata.c | 28 ++
> arch/riscv/include/asm/bugs.h| 22 +++
> arch/riscv/include/asm/errata_list.h | 3 +-
> arch/riscv/kernel/Makefile | 2 ++
> arch/riscv/kernel/bugs.c | 55
>
> arch/riscv/kernel/cpufeature.c | 9 +-
> drivers/base/cpu.c | 3 ++
> include/linux/cpu.h | 1 +
> 9 files changed, 132 insertions(+), 2 deletions(-)
>
> diff --git a/arch/riscv/Kconfig.errata b/arch/riscv/Kconfig.errata
> index 2acc7d876e1f..e318119d570d 100644
> --- a/arch/riscv/Kconfig.errata
> +++ b/arch/riscv/Kconfig.errata
> @@ -119,4 +119,15 @@ config ERRATA_THEAD_PMU
>
> If you don't know what to do here, say "Y".
>
> +config ERRATA_THEAD_GHOSTWRITE
> + bool "Apply T-Head Ghostwrite errata"
> + depends on ERRATA_THEAD && RISCV_ISA_XTHEADVECTOR
> + default y
> + help
> + The T-Head C9xx cores have a vulnerability in the xtheadvector
> + instruction set. When this errata is enabled, the CPUs will be probed
> + to determine if they are vulnerable and disable xtheadvector.
> +
> + If you don't know what to do here, say "Y".
> +
> endmenu # "CPU errata selection"
> diff --git a/arch/riscv/errata/thead/errata.c
> b/arch/riscv/errata/thead/errata.c
> index f5120e07c318..5cc008ab41a8 100644
> --- a/arch/riscv/errata/thead/errata.c
> +++ b/arch/riscv/errata/thead/errata.c
> @@ -10,6 +10,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -142,6 +143,31 @@ static bool errata_probe_pmu(unsigned int stage,
> return true;
> }
>
> +static bool errata_probe_ghostwrite(unsigned int stage,
> + unsigned long arch_id, unsigned long impid)
> +{
> + if (!IS_ENABLED(CONFIG_ERRATA_THEAD_GHOSTWRITE))
> + return false;
> +
> + /*
> + * target-c9xx cores report arch_id and impid as 0
> + *
> + * While ghostwrite may not affect all c9xx cores that implement
> + * xtheadvector, there is no futher granularity than c9xx. Assume
> + * vulnerable for this entire class of processors when xtheadvector is
> + * enabled.
> + */
Is it not possible to use the cpu compatible string for this? Given that
we only know if xtheadvector is enabled once we are already parsing the
cpu node devicetree, it seems, to me, as if it should be possible to be
more granular. AFAIU, some T-Head c900 series devices are not venerable.
Cheers,
Conor.
> + if (arch_id != 0 || impid != 0)
> + return false;
> +
> + if (stage != RISCV_ALTERNATIVES_EARLY_BOOT)
> + return false;
> +
> + ghostwrite_set_vulnerable();
> +
> + return true;
> +}
> +
> static u32 thead_errata_probe(unsigned int stage,
> unsigned long archid, unsigned long impid)
> {
> @@ -155,6 +181,8 @@ static u32 thead_errata_probe(unsigned int stage,
> if (errata_probe_pmu(stage, archid, impid))
> cpu_req_errata |= BIT(ERRATA_THEAD_PMU);
>
> + errata_probe_ghostwrite(stage, archid, impid);
> +
> return cpu_req_errata;
> }
>
> diff --git a/arch/riscv/include/asm/bugs.h b/arch/riscv/include/asm/bugs.h
> new file mode 100644
> index ..e294b15bf78e
> --- /dev/null
> +++ b/arch/riscv/include/asm/bugs.h
> @@ -0,0 +1,22 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Interface for managing mitigations for riscv vulnerabilities.
> + *
> + * Copyright (C) 2024 Rivos Inc.
> + */
> +
> +#ifndef __ASM_BUGS_H
> +#define __ASM_BUGS_H
> +
> +/* Watch out, ordering is important here. */
> +enum mitigation_state {
> + UNAFFECTED,
> + MITIGATED,
> + VULNERABLE,
> +};
> +
> +void ghostwrite_set_vulnerable(void);
> +void ghostwrite_enable_mitigation(void);
> +enum mitigation_state ghostwrite_get_state(void);
> +
> +#endif /* __ASM_BUGS_H */
> diff --git a/arch/riscv/include/asm/errata_list.h
> b/arch/riscv/include/asm/errata_list.h
> index 7c8a71a526a3..6e426ed7919a 100644
> --- a/arch/riscv/include/asm/errata_list.h
> +++ b/arch/riscv/include/asm/errata_list.h
> @@ -25,7 +25,8 @@
> #ifdef CONFIG_ERRATA_THEAD
> #define ERRATA_THEAD_MAE 0
> #define ERRATA_THEAD_PMU 1
> -#define ERRATA_THEAD_NUMBER 2
> +#define ERRATA_THEAD_GHOSTWRITE 2
> +#define ER
Re: [PATCH v10 14/14] riscv: Add ghostwrite vulnerability
On Mon, Sep 16, 2024 at 06:12:04PM +0100, Conor Dooley wrote:
> On Wed, Sep 11, 2024 at 10:55:22PM -0700, Charlie Jenkins wrote:
> > Follow the patterns of the other architectures that use
> > GENERIC_CPU_VULNERABILITIES for riscv to introduce the ghostwrite
> > vulnerability and mitigation. The mitigation is to disable all vector
> > which is accomplished by clearing the bit from the cpufeature field.
> >
> > Ghostwrite only affects thead c9xx CPUs that impelment xtheadvector, so
> > the vulerability will only be mitigated on these CPUs.
> >
> > Signed-off-by: Charlie Jenkins
> > ---
> > arch/riscv/Kconfig.errata| 11
> > arch/riscv/errata/thead/errata.c | 28 ++
> > arch/riscv/include/asm/bugs.h| 22 +++
> > arch/riscv/include/asm/errata_list.h | 3 +-
> > arch/riscv/kernel/Makefile | 2 ++
> > arch/riscv/kernel/bugs.c | 55
> >
> > arch/riscv/kernel/cpufeature.c | 9 +-
> > drivers/base/cpu.c | 3 ++
> > include/linux/cpu.h | 1 +
> > 9 files changed, 132 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/riscv/Kconfig.errata b/arch/riscv/Kconfig.errata
> > index 2acc7d876e1f..e318119d570d 100644
> > --- a/arch/riscv/Kconfig.errata
> > +++ b/arch/riscv/Kconfig.errata
> > @@ -119,4 +119,15 @@ config ERRATA_THEAD_PMU
> >
> > If you don't know what to do here, say "Y".
> >
> > +config ERRATA_THEAD_GHOSTWRITE
> > + bool "Apply T-Head Ghostwrite errata"
> > + depends on ERRATA_THEAD && RISCV_ISA_XTHEADVECTOR
> > + default y
> > + help
> > + The T-Head C9xx cores have a vulnerability in the xtheadvector
> > + instruction set. When this errata is enabled, the CPUs will be probed
> > + to determine if they are vulnerable and disable xtheadvector.
> > +
> > + If you don't know what to do here, say "Y".
> > +
> > endmenu # "CPU errata selection"
> > diff --git a/arch/riscv/errata/thead/errata.c
> > b/arch/riscv/errata/thead/errata.c
> > index f5120e07c318..5cc008ab41a8 100644
> > --- a/arch/riscv/errata/thead/errata.c
> > +++ b/arch/riscv/errata/thead/errata.c
> > @@ -10,6 +10,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -142,6 +143,31 @@ static bool errata_probe_pmu(unsigned int stage,
> > return true;
> > }
> >
> > +static bool errata_probe_ghostwrite(unsigned int stage,
> > + unsigned long arch_id, unsigned long impid)
> > +{
> > + if (!IS_ENABLED(CONFIG_ERRATA_THEAD_GHOSTWRITE))
> > + return false;
> > +
> > + /*
> > +* target-c9xx cores report arch_id and impid as 0
> > +*
> > +* While ghostwrite may not affect all c9xx cores that implement
> > +* xtheadvector, there is no futher granularity than c9xx. Assume
> > +* vulnerable for this entire class of processors when xtheadvector is
> > +* enabled.
> > +*/
>
> Is it not possible to use the cpu compatible string for this? Given that
> we only know if xtheadvector is enabled once we are already parsing the
> cpu node devicetree, it seems, to me, as if it should be possible to be
> more granular. AFAIU, some T-Head c900 series devices are not venerable.
Sure we can do that. I figured that since T-Head didn't feel it was
valuable to change the archid/implid between cores that Linux shouldn't
go out of its way to fix the granularity issue. Since you think it is
worthwhile though, I can try to work around this hardware issue.
- Charlie
>
> Cheers,
> Conor.
>
> > + if (arch_id != 0 || impid != 0)
> > + return false;
> > +
> > + if (stage != RISCV_ALTERNATIVES_EARLY_BOOT)
> > + return false;
> > +
> > + ghostwrite_set_vulnerable();
> > +
> > + return true;
> > +}
> > +
> > static u32 thead_errata_probe(unsigned int stage,
> > unsigned long archid, unsigned long impid)
> > {
> > @@ -155,6 +181,8 @@ static u32 thead_errata_probe(unsigned int stage,
> > if (errata_probe_pmu(stage, archid, impid))
> > cpu_req_errata |= BIT(ERRATA_THEAD_PMU);
> >
> > + errata_probe_ghostwrite(stage, archid, impid);
> > +
> > return cpu_req_errata;
> > }
> >
> > diff --git a/arch/riscv/include/asm/bugs.h b/arch/riscv/include/asm/bugs.h
> > new file mode 100644
> > index ..e294b15bf78e
> > --- /dev/null
> > +++ b/arch/riscv/include/asm/bugs.h
> > @@ -0,0 +1,22 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Interface for managing mitigations for riscv vulnerabilities.
> > + *
> > + * Copyright (C) 2024 Rivos Inc.
> > + */
> > +
> > +#ifndef __ASM_BUGS_H
> > +#define __ASM_BUGS_H
> > +
> > +/* Watch out, ordering is important here. */
> > +enum mitigation_state {
> > + UNAFFECTED,
> > + MITIGATED,
> > + VULNERABLE,
> > +};
> > +
> > +void ghostwrite_set_vulnerable(void);
> > +void ghostwrite_enable_mitiga
Re: [PATCH v10 14/14] riscv: Add ghostwrite vulnerability
On Mon, Sep 16, 2024 at 11:44:04AM -0700, Charlie Jenkins wrote: > I figured that since T-Head didn't feel it was > valuable to change the archid/implid between cores that Linux shouldn't > go out of its way to fix the granularity issue. I mean, when you put it like that, I'm not particularly inclined to disagree... signature.asc Description: PGP signature
Re: [PATCH v4 23/30] riscv signal: save and restore of shadow stack for signal
On Fri, Sep 13, 2024 at 09:25:57PM +0200, Andy Chiu wrote:
Hi Deepak,
Deepak Gupta 於 2024年9月13日 週五 上午1:20寫道：
Save shadow stack pointer in sigcontext structure while delivering signal.
Restore shadow stack pointer from sigcontext on sigreturn.
As part of save operation, kernel uses `ssamoswap` to save snapshot of
current shadow stack on shadow stack itself (can be called as a save
token). During restore on sigreturn, kernel retrieves token from top of
shadow stack and validates it. This allows that user mode can't arbitrary
pivot to any shadow stack address without having a token and thus provide
strong security assurance between signaly delivery and sigreturn window.
Signed-off-by: Deepak Gupta
Suggested-by: Andy Chiu
---
arch/riscv/include/asm/usercfi.h | 19 ++
arch/riscv/kernel/signal.c | 62 +++-
arch/riscv/kernel/usercfi.c | 57 +
3 files changed, 137 insertions(+), 1 deletion(-)
diff --git a/arch/riscv/include/asm/usercfi.h b/arch/riscv/include/asm/usercfi.h
index 20a9102cce51..d5050a5df26c 100644
--- a/arch/riscv/include/asm/usercfi.h
+++ b/arch/riscv/include/asm/usercfi.h
@@ -8,6 +8,7 @@
#ifndef __ASSEMBLY__
#include
#include
+#include
struct task_struct;
struct kernel_clone_args;
@@ -35,6 +36,9 @@ bool is_shstk_locked(struct task_struct *task);
bool is_shstk_allocated(struct task_struct *task);
void set_shstk_lock(struct task_struct *task);
void set_shstk_status(struct task_struct *task, bool enable);
+unsigned long get_active_shstk(struct task_struct *task);
+int restore_user_shstk(struct task_struct *tsk, unsigned long shstk_ptr);
+int save_user_shstk(struct task_struct *tsk, unsigned long *saved_shstk_ptr);
bool is_indir_lp_enabled(struct task_struct *task);
bool is_indir_lp_locked(struct task_struct *task);
void set_indir_lp_status(struct task_struct *task, bool enable);
@@ -96,6 +100,21 @@ static inline void set_shstk_status(struct task_struct
*task, bool enable)
}
+static inline int restore_user_shstk(struct task_struct *tsk, unsigned long
shstk_ptr)
+{
+ return -EINVAL;
+}
+
+static inline int save_user_shstk(struct task_struct *tsk, unsigned long
*saved_shstk_ptr)
+{
+ return -EINVAL;
+}
+
+static inline unsigned long get_active_shstk(struct task_struct *task)
+{
+ return 0;
+}
+
static inline bool is_indir_lp_enabled(struct task_struct *task)
{
return false;
diff --git a/arch/riscv/kernel/signal.c b/arch/riscv/kernel/signal.c
index dcd282419456..7d5c1825650f 100644
--- a/arch/riscv/kernel/signal.c
+++ b/arch/riscv/kernel/signal.c
@@ -22,6 +22,7 @@
#include
#include
#include
+#include
unsigned long signal_minsigstksz __ro_after_init;
@@ -153,6 +154,16 @@ static long restore_sigcontext(struct pt_regs *regs,
void __user *sc_ext_ptr = &sc->sc_extdesc.hdr;
__u32 rsvd;
long err;
+ unsigned long ss_ptr = 0;
+ struct __sc_riscv_cfi_state __user *sc_cfi = NULL;
+
+ sc_cfi = (struct __sc_riscv_cfi_state *)
+((unsigned long) sc_ext_ptr + sizeof(struct __riscv_ctx_hdr));
+
+ if (has_vector() && riscv_v_vstate_query(regs))
+ sc_cfi = (struct __sc_riscv_cfi_state *)
+((unsigned long) sc_cfi + riscv_v_sc_size);
+
/* sc_regs is structured the same as the start of pt_regs */
err = __copy_from_user(regs, &sc->sc_regs, sizeof(sc->sc_regs));
if (unlikely(err))
@@ -172,6 +183,24 @@ static long restore_sigcontext(struct pt_regs *regs,
if (unlikely(rsvd))
return -EINVAL;
+ /*
+* Restore shadow stack as a form of token stored on shadow stack
itself as a safe
+* way to restore.
+* A token on shadow gives following properties
+* - Safe save and restore for shadow stack switching. Any save of
shadow stack
+*must have had saved a token on shadow stack. Similarly any
restore of shadow
+*stack must check the token before restore. Since writing to
shadow stack with
+*address of shadow stack itself is not easily allowed. A
restore without a save
+*is quite difficult for an attacker to perform.
+* - A natural break. A token in shadow stack provides a natural
break in shadow stack
+*So a single linear range can be bucketed into different
shadow stack segments.
+*sspopchk will detect the condition and fault to kernel as sw
check exception.
+*/
+ if (is_shstk_enabled(current)) {
+ err |= __copy_from_user(&ss_ptr, &sc_cfi->ss_ptr,
sizeof(unsigned long));
+ err |= restore_user_shstk(current, ss_ptr);
+ }
+
while (!err) {
__u32 magic, size;
struct __riscv_ctx_hdr __user *head = sc_ext_ptr;
@@ -215,6 +244,10 @@ static size_t get_rt_frame_size(bool cal_all)
if (cal_all || riscv
Re: [PATCH v4 21/30] riscv/traps: Introduce software check exception
On Fri, Sep 13, 2024 at 09:35:50PM +0200, Andy Chiu wrote:
Hi Deepak
Deepak Gupta 於 2024年9月13日 週五 上午2:32寫道：
zicfiss / zicfilp introduces a new exception to priv isa `software check
exception` with cause code = 18. This patch implements software check
exception.
Additionally it implements a cfi violation handler which checks for code
in xtval. If xtval=2, it means that sw check exception happened because of
an indirect branch not landing on 4 byte aligned PC or not landing on
`lpad` instruction or label value embedded in `lpad` not matching label
value setup in `x7`. If xtval=3, it means that sw check exception happened
because of mismatch between link register (x1 or x5) and top of shadow
stack (on execution of `sspopchk`).
In case of cfi violation, SIGSEGV is raised with code=SEGV_CPERR.
SEGV_CPERR was introduced by x86 shadow stack patches.
Signed-off-by: Deepak Gupta
---
arch/riscv/include/asm/asm-prototypes.h | 1 +
arch/riscv/include/asm/entry-common.h | 2 ++
arch/riscv/kernel/entry.S | 3 ++
arch/riscv/kernel/traps.c | 38 +
4 files changed, 44 insertions(+)
diff --git a/arch/riscv/include/asm/asm-prototypes.h
b/arch/riscv/include/asm/asm-prototypes.h
index cd627ec289f1..5a27cefd7805 100644
--- a/arch/riscv/include/asm/asm-prototypes.h
+++ b/arch/riscv/include/asm/asm-prototypes.h
@@ -51,6 +51,7 @@ DECLARE_DO_ERROR_INFO(do_trap_ecall_u);
DECLARE_DO_ERROR_INFO(do_trap_ecall_s);
DECLARE_DO_ERROR_INFO(do_trap_ecall_m);
DECLARE_DO_ERROR_INFO(do_trap_break);
+DECLARE_DO_ERROR_INFO(do_trap_software_check);
asmlinkage void handle_bad_stack(struct pt_regs *regs);
asmlinkage void do_page_fault(struct pt_regs *regs);
diff --git a/arch/riscv/include/asm/entry-common.h
b/arch/riscv/include/asm/entry-common.h
index 2293e535f865..4068c7e5452a 100644
--- a/arch/riscv/include/asm/entry-common.h
+++ b/arch/riscv/include/asm/entry-common.h
@@ -39,4 +39,6 @@ static inline int handle_misaligned_store(struct pt_regs
*regs)
}
#endif
+bool handle_user_cfi_violation(struct pt_regs *regs);
+
#endif /* _ASM_RISCV_ENTRY_COMMON_H */
diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
index ca9203e6d76d..2ec75ba864a8 100644
--- a/arch/riscv/kernel/entry.S
+++ b/arch/riscv/kernel/entry.S
@@ -384,6 +384,9 @@ SYM_DATA_START_LOCAL(excp_vect_table)
RISCV_PTR do_page_fault /* load page fault */
RISCV_PTR do_trap_unknown
RISCV_PTR do_page_fault /* store page fault */
+ RISCV_PTR do_trap_unknown /* cause=16 */
+ RISCV_PTR do_trap_unknown /* cause=17 */
+ RISCV_PTR do_trap_software_check /* cause=18 is sw check exception */
SYM_DATA_END_LABEL(excp_vect_table, SYM_L_LOCAL, excp_vect_table_end)
#ifndef CONFIG_MMU
diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
index 51ebfd23e007..32d1453bed72 100644
--- a/arch/riscv/kernel/traps.c
+++ b/arch/riscv/kernel/traps.c
@@ -354,6 +354,44 @@ void do_trap_ecall_u(struct pt_regs *regs)
}
+#define CFI_TVAL_FCFI_CODE 2
+#define CFI_TVAL_BCFI_CODE 3
+/* handle cfi violations */
+bool handle_user_cfi_violation(struct pt_regs *regs)
+{
+ bool ret = false;
+ unsigned long tval = csr_read(CSR_TVAL);
+
+ if (((tval == CFI_TVAL_FCFI_CODE) &&
cpu_supports_indirect_br_lp_instr()) ||
+ ((tval == CFI_TVAL_BCFI_CODE) && cpu_supports_shadow_stack())) {
+ do_trap_error(regs, SIGSEGV, SEGV_CPERR, regs->epc,
+ "Oops - control flow violation");
+ ret = true;
+ }
+
+ return ret;
+}
+/*
+ * software check exception is defined with risc-v cfi spec. Software check
+ * exception is raised when:-
+ * a) An indirect branch doesn't land on 4 byte aligned PC or `lpad`
+ *instruction or `label` value programmed in `lpad` instr doesn't
+ *match with value setup in `x7`. reported code in `xtval` is 2.
+ * b) `sspopchk` instruction finds a mismatch between top of shadow stack (ssp)
+ *and x1/x5. reported code in `xtval` is 3.
+ */
It seems like this trap handler does not follow generic entry. This
can cause problems as signal delivery is done in
irqentry_exit_to_user_mode(). Please reference the commit f0bddf50586d
("riscv: entry: Convert to generic entry") for more information.
Ack. will fix it.
+asmlinkage __visible __trap_section void do_trap_software_check(struct pt_regs
*regs)
+{
+ if (user_mode(regs)) {
+ /* not a cfi violation, then merge into flow of unknown trap
handler */
+ if (!handle_user_cfi_violation(regs))
+ do_trap_unknown(regs);
+ } else {
+ /* sw check exception coming from kernel is a bug in kernel */
+ die(regs, "Kernel BUG");
+ }
+}
+
#ifdef CONFIG_MMU
asmlinkage __visible noinstr void do_page_fault(struct pt_regs *regs)
{
--
2.45.0
___
linux-riscv mailing list
l
