NetBSD Problem Report #54655

From taca@back-street.net  Tue Oct 29 14:25:21 2019
Return-Path: <taca@back-street.net>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 77A647A1D3
	for <gnats-bugs@gnats.NetBSD.org>; Tue, 29 Oct 2019 14:25:21 +0000 (UTC)
Message-Id: <20191029142514.5B9D231FD6@edge.back-street.net>
Date: Tue, 29 Oct 2019 23:25:14 +0900 (JST)
From: taca@back-street.net
Reply-To: taca@back-street.net
To: gnats-bugs@NetBSD.org
Subject: cpu_rng_rdseed() should check support of RDRAND instruction
X-Send-Pr-Version: 3.95

>Number:         54655
>Category:       kern
>Synopsis:       cpu_rng_rdseed() should check support of RDRAND instruction
>Confidential:   no
>Severity:       critical
>Priority:       medium
>Responsible:    kern-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Oct 29 14:30:00 +0000 2019
>Closed-Date:    Fri Dec 13 16:31:37 +0000 2019
>Last-Modified:  Fri Dec 13 16:31:37 +0000 2019
>Originator:     Takahiro Kambe
>Release:        NetBSD 9.0_BETA
>Organization:
Takahiro Kambe
>Environment:


System: NetBSD 9.0_BETA (SAKURA-VPS4) #26: Tue Oct 29 00:41:42 JST 2019  taca@edge.back-street.net:/data/netbsd-9/amd64/obj/sys/arch/amd64/compile/SAKURA-VPS4
Architecture: x86_64
Machine: amd64
>Description:
cpu_rng_rdseed() in sys/arch/x86/x86/cpu_rng.c dose not honor support
of RDRAND instruction.

(1) In cpu_earlyrng(), support of RDSEED and RDRAND is checked with
    cpu_feature.

	bool has_rdseed = (cpu_feature[5] & CPUID_SEF_RDSEED) != 0;
	bool has_rdrand = (cpu_feature[1] & CPUID2_RDRAND) != 0;

(2) Then cpu_rng_rdseed() and cpu_rng_rdrand() is called with these
    results:

	if (has_rdseed) {
		for (i = 0; i < 8; i++) {
			if (cpu_rng_rdseed(&buf[i]) == 0) {
...
	} else if (has_rdrand) {
		for (i = 0; i < 8; i++) {
			if (cpu_rng_rdrand(&buf[i]) == 0) {
...

(3) But cpu_rng_rdseed(() may call cpu_rng_rdrand() even if the case of
    has_rdrand is zero.

static size_t
cpu_rng_rdseed(cpu_rng_t *out)
{
	uint8_t rndsts;
	...
	/*
	 * Userspace could have exhausted RDSEED, but the
	 * CPU-internal generator feeding RDRAND is guaranteed
	 * to be seeded even in this case.
	 */
exhausted:
	return cpu_rng_rdrand(out);
}

I don't know is there real CPU supports RDSEED without RDRAND or not.
But there is such a case on VPS service at least.

$ /usr/sbin/cpuctl identify 0 | egrep RD
cpu0: features2 0x28100800<SYSCALL/SYSRET,XD,RDTSCP,EM64T>
cpu0: features5 0x1c07a9<FSGSBASE,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,RDSEED,ADX>

I met this problem when updating a host running NetBSD 8.1_STABLE on a VPS
to 9.0_BETA.  After I reported it to NetBSD mailing list in Japanese
(http://www.re.soum.co.jp/~jun/welcome.html#netbsd), Azuma Okamoto told
where the problem (commeting out some lines make kernel work).

Thanks to Azuma Okamoto's examination what changed from NetBSD 8.  Without
his effort, I could not reach here.

>How-To-Repeat:
Boot NetBSD 9.0_BETA cdrom on a VPS (version 4) provided by
<https://vps.sakura.ad.jp>.

>Fix:
Check support of RDRAND in cpu_rng_rdseed():

diff --git a/sys/arch/x86/x86/cpu_rng.c b/sys/arch/x86/x86/cpu_rng.c
index 3b79d768ea..c716eeaa26 100644
--- a/sys/arch/x86/x86/cpu_rng.c
+++ b/sys/arch/x86/x86/cpu_rng.c
@@ -53,6 +53,8 @@ static enum {
 	CPU_RNG_VIA
 } cpu_rng_mode __read_mostly = CPU_RNG_NONE;

+static bool has_rdrand;
+
 bool
 cpu_rng_init(void)
 {
@@ -131,7 +133,10 @@ cpu_rng_rdseed(cpu_rng_t *out)
 	 * to be seeded even in this case.
 	 */
 exhausted:
-	return cpu_rng_rdrand(out);
+	if (has_rdrand)
+		return cpu_rng_rdrand(out);
+	else
+		return 0;
 }

 static size_t
@@ -213,7 +218,7 @@ cpu_earlyrng(void *out, size_t sz)
 	int i;

 	bool has_rdseed = (cpu_feature[5] & CPUID_SEF_RDSEED) != 0;
-	bool has_rdrand = (cpu_feature[1] & CPUID2_RDRAND) != 0;
+	has_rdrand = (cpu_feature[1] & CPUID2_RDRAND) != 0;

 	KASSERT(sz + sizeof(uint64_t) <= SHA512_DIGEST_LENGTH);


>Release-Note:

>Audit-Trail:
From: Masanobu SAITOH <msaitoh@execsw.org>
To: gnats-bugs@netbsd.org, kern-bug-people@netbsd.org,
 gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Cc: msaitoh@execsw.org
Subject: Re: kern/54655: cpu_rng_rdseed() should check support of RDRAND
 instruction
Date: Fri, 1 Nov 2019 16:51:37 +0900

 > I don't know is there real CPU supports RDSEED without RDRAND or not.

 I grepped InstLatx64(https://github.com/InstLatx64/InstLatx64)'s CPUID
 outputs and there is no any CPU who has RDSEED but not RDRAND.

 > But there is such a case on VPS service at least.

 Even though there is no such real CPU, we should check the existence
 individually because VM environment tend to drop some CPUID bits
 because of the security, so I think your proposal patch is OK.

 >> Fix:
 > Check support of RDRAND in cpu_rng_rdseed():
 > 
 > diff --git a/sys/arch/x86/x86/cpu_rng.c b/sys/arch/x86/x86/cpu_rng.c
 > index 3b79d768ea..c716eeaa26 100644
 > --- a/sys/arch/x86/x86/cpu_rng.c
 > +++ b/sys/arch/x86/x86/cpu_rng.c
 > @@ -53,6 +53,8 @@ static enum {
 >  	CPU_RNG_VIA
 >  } cpu_rng_mode __read_mostly = CPU_RNG_NONE;
 >  
 > +static bool has_rdrand;
 > +
 >  bool
 >  cpu_rng_init(void)
 >  {
 > @@ -131,7 +133,10 @@ cpu_rng_rdseed(cpu_rng_t *out)
 >  	 * to be seeded even in this case.
 >  	 */
 >  exhausted:
 > -	return cpu_rng_rdrand(out);
 > +	if (has_rdrand)
 > +		return cpu_rng_rdrand(out);
 > +	else
 > +		return 0;
 >  }
 >  
 >  static size_t
 > @@ -213,7 +218,7 @@ cpu_earlyrng(void *out, size_t sz)
 >  	int i;
 >  
 >  	bool has_rdseed = (cpu_feature[5] & CPUID_SEF_RDSEED) != 0;
 > -	bool has_rdrand = (cpu_feature[1] & CPUID2_RDRAND) != 0;
 > +	has_rdrand = (cpu_feature[1] & CPUID2_RDRAND) != 0;

 It's not related to this PR, we should avoid referring the boot
 processor's cpu_feature[] in future for Lakefield and Elkhart Lake.

 >  
 >  	KASSERT(sz + sizeof(uint64_t) <= SHA512_DIGEST_LENGTH);
 >  
 > 
 >> Unformatted:
 >  	
 >  	
 > 


 -- 
 -----------------------------------------------
                 SAITOH Masanobu (msaitoh@execsw.org
                                  msaitoh@netbsd.org)

State-Changed-From-To: open->pending-pullups
State-Changed-By: taca@NetBSD.org
State-Changed-When: Fri, 01 Nov 2019 15:08:15 +0000
State-Changed-Why:
I've commited patch by myself and requested pullup request.


State-Changed-From-To: pending-pullups->closed
State-Changed-By: taca@NetBSD.org
State-Changed-When: Fri, 13 Dec 2019 16:31:37 +0000
State-Changed-Why:
Applied to netbsd-9 branch.


>Unformatted:

Home	PR Database Search