[PATCH] RISC-V: KVM: Publish NACL HFENCE configuration last

[PATCH] RISC-V: KVM: Publish NACL HFENCE configuration last

[Pengpeng Hou]

The SBI NACL interface requires software to write Page_Number and
Page_Count before publishing an HFENCE entry with Config.Pending set.
__kvm_riscv_nacl_hfence() currently stores the pending configuration first.

Write the payload first and order those stores before publishing the
configuration word with WRITE_ONCE(). Use a write barrier that also
applies to UP kernels, since the consumer is outside the Linux SMP
domain.

Keep nacl_hfence_mkconfig() unchanged: setting Pending in a local value
does not publish the entry. The shared-memory store is the publication
point.

The issue was found by our static-analysis tool and manually reviewed.

Fixes: d466c19cead5 ("RISC-V: KVM: Add common nested acceleration support")
Assisted-by: GPT-5
Signed-off-by: Pengpeng Hou <hppiscas@163.com>
---

diff --git a/arch/riscv/kvm/nacl.c b/arch/riscv/kvm/nacl.c
index 9aff03c4..c5c4907a 100644
--- a/arch/riscv/kvm/nacl.c
+++ b/arch/riscv/kvm/nacl.c
@@ -5,6 +5,7 @@
 
 #include <linux/kvm_host.h>
 #include <linux/vmalloc.h>
+#include <asm/barrier.h>
 #include <asm/kvm_nacl.h>
 
 DEFINE_STATIC_KEY_FALSE(kvm_riscv_nacl_available);
@@ -42,12 +43,15 @@ again:
 		}
 	}
 
-	entp = shmem + SBI_NACL_SHMEM_HFENCE_ENTRY_CONFIG(i);
-	*entp = cpu_to_lelong(control);
 	entp = shmem + SBI_NACL_SHMEM_HFENCE_ENTRY_PNUM(i);
 	*entp = cpu_to_lelong(page_num);
 	entp = shmem + SBI_NACL_SHMEM_HFENCE_ENTRY_PCOUNT(i);
 	*entp = cpu_to_lelong(page_count);
+
+	/* Publish the payload before setting Config.Pending. */
+	wmb();
+	entp = shmem + SBI_NACL_SHMEM_HFENCE_ENTRY_CONFIG(i);
+	WRITE_ONCE(*entp, cpu_to_lelong(control));
 }
 
 int kvm_riscv_nacl_enable(void)

base-commit: 4d7d9486c04d917265f64c55bd23b2cc4fe7749c

Re: [PATCH] RISC-V: KVM: Publish NACL HFENCE configuration last

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] KVM silently drops TLB flush (HFENCE) requests when the NACL shared memory is exhausted, leading to guest memory corruption and use-after-free vulnerabilities.
- [High] Per-CPU `nacl_shmem()` is evaluated and accessed in a preemptible context, causing a race condition that corrupts shared memory entries and drops TLB flushes.
- [High] The shared memory entry configuration is read without `READ_ONCE()`, violating Linux Kernel Memory Model (LKMM) rules for shared memory and control dependencies.
--

commit c8294fdfde274970a69be416dee2db8089a33132
Author: Pengpeng Hou <hppiscas@163.com>
Subject: RISC-V: KVM: Publish NACL HFENCE configuration last

The SBI NACL interface requires software to write Page_Number and
Page_Count before publishing an HFENCE entry with Config.Pending set.
__kvm_riscv_nacl_hfence() currently stores the pending configuration first.

> diff --git a/arch/riscv/kvm/nacl.c b/arch/riscv/kvm/nacl.c
> index 9aff03c4f667a..c5c4907aca08a 100644
> --- a/arch/riscv/kvm/nacl.c
> +++ b/arch/riscv/kvm/nacl.c
> @@ -5,6 +5,7 @@
>  
>  #include <linux/kvm_host.h>
>  #include <linux/vmalloc.h>
> +#include <asm/barrier.h>
>  #include <asm/kvm_nacl.h>
>  
>  DEFINE_STATIC_KEY_FALSE(kvm_riscv_nacl_available);
> @@ -42,12 +43,15 @@ void __kvm_riscv_nacl_hfence(void *shmem,

[Severity: High]
This isn't a bug introduced by this patch, but in __kvm_riscv_nacl_hfence(),
KVM silently drops TLB flush requests when the NACL shared memory fills up.

When the shared memory queue fills up, the function calls
nacl_sync_hfence(-1UL) and retries 5 times. If the firmware is sluggish or
fails to drain the queue in time, it returns void without notifying the
caller:

arch/riscv/kvm/nacl.c:__kvm_riscv_nacl_hfence() {
    ...
	if (ent < 0) {
		if (try_count--) {
			nacl_sync_hfence(-1UL);
			goto again;
		} else {
			pr_warn("KVM: No free entry in NACL shared memory\n");
			return;
		}
	}
    ...
}

Could KVM return a hard error or BUG() instead if a TLB flush cannot be
completed, to prevent guests from accessing stale physical memory mappings
and causing data corruption or use-after-free?

[Severity: High]
This is a pre-existing issue, but does reading the shared memory entry
configuration without READ_ONCE() violate the Linux Kernel Memory Model for
shared memory and control dependencies?

arch/riscv/kvm/nacl.c:__kvm_riscv_nacl_hfence() {
    ...
again:
	for (i = 0; i < SBI_NACL_SHMEM_HFENCE_ENTRY_MAX; i++) {
		entp = shmem + SBI_NACL_SHMEM_HFENCE_ENTRY_CONFIG(i);
		if (lelong_to_cpu(*entp) & SBI_NACL_SHMEM_HFENCE_CONFIG_PEND)
			continue;
    ...
}

While this patch correctly adds a WRITE_ONCE() and a barrier for publishing
the payload, the read side is left unprotected. Without READ_ONCE() around
*entp, might the compiler optimize the read from shared memory by caching the
value or issuing multiple loads, which could result in an infinite loop or
incorrect synchronization with the firmware?

>  		}
>  	}
>  
> -	entp = shmem + SBI_NACL_SHMEM_HFENCE_ENTRY_CONFIG(i);
> -	*entp = cpu_to_lelong(control);
>  	entp = shmem + SBI_NACL_SHMEM_HFENCE_ENTRY_PNUM(i);
>  	*entp = cpu_to_lelong(page_num);
>  	entp = shmem + SBI_NACL_SHMEM_HFENCE_ENTRY_PCOUNT(i);
>  	*entp = cpu_to_lelong(page_count);
> +
> +	/* Publish the payload before setting Config.Pending. */
> +	wmb();
> +	entp = shmem + SBI_NACL_SHMEM_HFENCE_ENTRY_CONFIG(i);
> +	WRITE_ONCE(*entp, cpu_to_lelong(control));
>  }

[Severity: High]
This isn't a bug introduced by this patch, but is it safe to evaluate and
access the per-CPU nacl_shmem() in a preemptible context?

In kvm_arch_vcpu_ioctl_run(), vCPU requests are processed before preemption
is disabled:

arch/riscv/kvm/vcpu.c:kvm_arch_vcpu_ioctl_run() {
    ...
		ret = kvm_riscv_check_vcpu_requests(vcpu);
		if (ret <= 0)
			continue;

		preempt_disable();
    ...
}

When processing HFENCE requests, kvm_riscv_hfence_process() evaluates
nacl_shmem(), which expands to a per-CPU pointer:

arch/riscv/include/asm/kvm_nacl.h:
#define nacl_shmem()							\
	this_cpu_ptr(&kvm_riscv_nacl)->shmem

arch/riscv/kvm/tlb.c:kvm_riscv_hfence_process() {
    ...
				nacl_hfence_gvma_vmid(nacl_shmem(), d.vmid,
						      d.addr, d.size, d.order);
    ...
}

If the vCPU thread is preempted and migrated to another CPU after evaluating
nacl_shmem(), wouldn't this cause a race condition that overwrites another
CPU's TLB flush queue, potentially leading to dropped flushes and guest
isolation breaches?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/caplitmus-rv.c91c005b844342d9a9f60949b04c520d.d65@163.com?part=1

Re: [PATCH] RISC-V: KVM: Publish NACL HFENCE configuration last

[Anup Patel]

On Sun, Sep 6, 2026 at 1:37 PM Pengpeng Hou <hppiscas@163.com> wrote:
>
> The SBI NACL interface requires software to write Page_Number and
> Page_Count before publishing an HFENCE entry with Config.Pending set.
> __kvm_riscv_nacl_hfence() currently stores the pending configuration first.
>
> Write the payload first and order those stores before publishing the
> configuration word with WRITE_ONCE(). Use a write barrier that also
> applies to UP kernels, since the consumer is outside the Linux SMP
> domain.
>
> Keep nacl_hfence_mkconfig() unchanged: setting Pending in a local value
> does not publish the entry. The shared-memory store is the publication
> point.
>
> The issue was found by our static-analysis tool and manually reviewed.
>
> Fixes: d466c19cead5 ("RISC-V: KVM: Add common nested acceleration support")
> Assisted-by: GPT-5
> Signed-off-by: Pengpeng Hou <hppiscas@163.com>

There was another patch from Zongmin Zhou which is relatively
more complete hence I have merged that one.

Regards,
Anup


> ---
>
> diff --git a/arch/riscv/kvm/nacl.c b/arch/riscv/kvm/nacl.c
> index 9aff03c4..c5c4907a 100644
> --- a/arch/riscv/kvm/nacl.c
> +++ b/arch/riscv/kvm/nacl.c
> @@ -5,6 +5,7 @@
>
>  #include <linux/kvm_host.h>
>  #include <linux/vmalloc.h>
> +#include <asm/barrier.h>
>  #include <asm/kvm_nacl.h>
>
>  DEFINE_STATIC_KEY_FALSE(kvm_riscv_nacl_available);
> @@ -42,12 +43,15 @@ again:
>                 }
>         }
>
> -       entp = shmem + SBI_NACL_SHMEM_HFENCE_ENTRY_CONFIG(i);
> -       *entp = cpu_to_lelong(control);
>         entp = shmem + SBI_NACL_SHMEM_HFENCE_ENTRY_PNUM(i);
>         *entp = cpu_to_lelong(page_num);
>         entp = shmem + SBI_NACL_SHMEM_HFENCE_ENTRY_PCOUNT(i);
>         *entp = cpu_to_lelong(page_count);
> +
> +       /* Publish the payload before setting Config.Pending. */
> +       wmb();
> +       entp = shmem + SBI_NACL_SHMEM_HFENCE_ENTRY_CONFIG(i);
> +       WRITE_ONCE(*entp, cpu_to_lelong(control));
>  }
>
>  int kvm_riscv_nacl_enable(void)
>
> base-commit: 4d7d9486c04d917265f64c55bd23b2cc4fe7749c
>