Re: [PATCH v7 1/2] drivers/misc: sysgenid: add system generation id driver

From: "MacCarthaigh, Colm" <colmmacc@amazon.com>
To: "Michael S. Tsirkin" <mst@redhat.com>, "Graf (AWS),
	Alexander" <graf@amazon.de>
Cc: "Catangiu, Adrian Costin" <acatan@amazon.com>,
	"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"linux-s390@vger.kernel.org" <linux-s390@vger.kernel.org>,
	"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
	"rdunlap@infradead.org" <rdunlap@infradead.org>,
	"arnd@arndb.de" <arnd@arndb.de>,
	"ebiederm@xmission.com" <ebiederm@xmission.com>,
	"rppt@kernel.org" <rppt@kernel.org>,
	"0x7f454c46@gmail.com" <0x7f454c46@gmail.com>,
	"borntraeger@de.ibm.com" <borntraeger@de.ibm.com>,
	"Jason@zx2c4.com" <Jason@zx2c4.com>,
	"jannh@google.com" <jannh@google.com>, "w@1wt.eu" <w@1wt.eu>,
	"luto@kernel.org" <luto@kernel.org>,
	"tytso@mit.edu" <tytso@mit.edu>,
	"ebiggers@kernel.org" <ebiggers@kernel.org>,
	"Woodhouse, David" <dwmw@amazon.co.uk>,
	"bonzini@gnu.org" <bonzini@gnu.org>,
	"Singh, Balbir" <sblbir@amazon.com>,
	"Weiss, Radu" <raduweis@amazon.com>,
	"corbet@lwn.net" <corbet@lwn.net>,
	"mhocko@kernel.org" <mhocko@kernel.org>,
	"rafael@kernel.org" <rafael@kernel.org>,
	"pavel@ucw.cz" <pavel@ucw.cz>,
	"mpe@ellerman.id.au" <mpe@ellerman.id.au>,
	"areber@redhat.com" <areber@redhat.com>,
	"ovzxemul@gmail.com" <ovzxemul@gmail.com>,
	"avagin@gmail.com" <avagin@gmail.com>,
	"ptikhomirov@virtuozzo.com" <ptikhomirov@virtuozzo.com>,
	"gil@azul.com" <gil@azul.com>,
	"asmehra@redhat.com" <asmehra@redhat.com>,
	"dgunigun@redhat.com" <dgunigun@redhat.com>,
	"vijaysun@ca.ibm.com" <vijaysun@ca.ibm.com>,
	"oridgar@gmail.com" <oridgar@gmail.com>,
	"ghammer@redhat.com" <ghammer@redhat.com>
Subject: Re: [PATCH v7 1/2] drivers/misc: sysgenid: add system generation id driver
Date: Wed, 24 Feb 2021 23:00:46 +0000	[thread overview]
Message-ID: <7DE31E14-D6E9-41EA-9A43-6608ACC7CD87@amazon.com> (raw)

On 2/24/21, 2:44 PM, "Michael S. Tsirkin" <mst@redhat.com> wrote:
    > The mmap mechanism allows the PRNG to reseed after a genid change. Because
    > we don't have an event mechanism for this code path, that can happen minutes
    > after the resume. But that's ok, we "just" have to ensure that nobody is
    > consuming secret data at the point of the snapshot.

    Something I am still not clear on is whether it's really important to
    skip the system call here. If not I think it's prudent to just stick
    to read for now, I think there's a slightly lower chance that
    it will get misused. mmap which gives you a laggy gen id value
    really seems like it would be hard to use correctly.

It's not uncommon for these user-space PRNGs to used quite a lot in very performance critical paths. If you negotiate a TLS session that uses an explicit IV, the RNG is being called for every TLS record sent. Same for IPSec depending on the cipher-suite. Every TLS hello message has 28-32 bytes of data from the RNG, or if you've got ECDSA as your signature algorithm, it's inline again. Using RSA_PSS? Same again. Many Post-Quantum algorithms are even more veraciously entropy hungry.  We examine the compiled instructions for ours by hand to check it's all as tight as it can be. 

To give more of an idea, several crypto libraries took out the getpid() guards they had for fork detection in the RNGs, though VDSO could have helped there and I'm not sure they would have needed to if VDSO were more widely used at the time.  I don't think we'd get a patch into OpenSSL/libcrypto that involves a full syscall. VDSO might be ok, but even that's not going to have the speed that a single memory lookup can do with the mmap/madvise approach ... since we already have to use WIPEONFORK.

In practice I don't think it will be that hard to use correctly; snapshots and restores of this nature really have to happen only when the activity is quiescent. If operations are in-flight, it's not easy to reason about the potential multi-restore problems at all and it only makes sense to think about transactional correctness at the level of all transactions that may have been in-flight. The mmap solution is more about integrating with existing library APIs and semantics than it is about somehow solving that at the kernel level. That part has to be solved at the system level.

- 
Colm