linux-kernel - Re: propagating vmgenid outward and upward

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 1 Mar 2022 17:35:25 +0100
From:   "Jason A. Donenfeld" <Jason@...c4.com>
To:     "Michael S. Tsirkin" <mst@...hat.com>
Cc:     linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        qemu-devel@...gnu.org, linux-hyperv@...r.kernel.org,
        linux-crypto@...r.kernel.org, graf@...zon.com,
        mikelley@...rosoft.com, gregkh@...uxfoundation.org,
        adrian@...ity.io, lersek@...hat.com, berrange@...hat.com,
        linux@...inikbrodowski.net, jannh@...gle.com, rafael@...nel.org,
        len.brown@...el.com, pavel@....cz, linux-pm@...r.kernel.org,
        colmmacc@...zon.com, tytso@....edu, arnd@...db.de
Subject: Re: propagating vmgenid outward and upward

Hi Michael,

On Tue, Mar 01, 2022 at 11:21:38AM -0500, Michael S. Tsirkin wrote:
> > If we had a "pull" model, rather than just expose a 16-byte unique
> > identifier, the vmgenid virtual hardware would _also_ expose a
> > word-sized generation counter, which would be incremented every time the
> > unique ID changed. Then, every time we would touch the RNG, we'd simply
> > do an inexpensive check of this memremap()'d integer, and reinitialize
> > with the unique ID if the integer changed. In this way, the race would
> > be entirely eliminated. We would then be able to propagate this outwards
> > to other drivers, by just exporting an extern symbol, in the manner of
> > `jiffies`, and propagate it upwards to userspace, by putting it in the
> > vDSO, in the manner of gettimeofday. And like that, there'd be no
> > terrible async thing and things would work pretty easily.
> 
> I am not sure what the difference is though. So we have a 16 byte unique
> value and you would prefer a dword counter. How is the former not a
> superset of the later?  

Laszlo just asked the same question, which I answered here:
<https://lore.kernel.org/lkml/Yh5JwK6toc%2FzBNL7@zx2c4.com/>. You have
to read the full 16 bytes. You can't safely just read the first 4 or 8
or something, because it's a "unique ID" rather than a counter. That
seems like a needlessly expensive thing to do on each-and-every packet.

> I'm not sure how safe it is to expose it to
> userspace specifically, but rest of text talks about exposing it to a
> kernel driver so maybe not an issue? So what makes interrupt driven
> required, and why not just remap and read existing vmgenid in the pull
> manner?  What did I miss?

I don't really understand your question, but guessing your meaning: I'm
not talking about exposing the actual 16-byte value to any other
drivers, but just notifying them that their sessions should be dropped.
If it's easier to think about this in code, grep for wg_pm_notification(),
and consider that it'd be changing this code:

        if (action != PM_HIBERNATION_PREPARE && action != PM_SUSPEND_PREPARE)
                return 0;

into:

        if (action != PM_HIBERNATION_PREPARE && action != PM_SUSPEND_PREPARE &&
	    action != PM_VMFORK_POST)
                return 0;

But perhaps I misunderstood this part of your question?

Jason