Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 1 Dec 2022 03:16:20 +0100
From:   "Jason A. Donenfeld" <Jason@zx2c4.com>
To:     Florian Weimer <fweimer@redhat.com>
Cc:     linux-kernel@vger.kernel.org, patches@lists.linux.dev,
        tglx@linutronix.de, linux-crypto@vger.kernel.org,
        linux-api@vger.kernel.org, x86@kernel.org,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>,
        Carlos O'Donell <carlos@redhat.com>,
        Arnd Bergmann <arnd@arndb.de>,
        Christian Brauner <brauner@kernel.org>
Subject: Re: [PATCH v10 1/4] random: add vgetrandom_alloc() syscall
Message-ID: <Y4gOdC8J+zzRsago@zx2c4.com>
References: <20221129210639.42233-1-Jason@zx2c4.com>
 <20221129210639.42233-2-Jason@zx2c4.com>
 <877czc7m0g.fsf@oldenburg.str.redhat.com>
 <Y4d5SyU3akA9ZBaJ@zx2c4.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <Y4d5SyU3akA9ZBaJ@zx2c4.com>
Precedence: bulk

On Wed, Nov 30, 2022 at 04:39:55PM +0100, Jason A. Donenfeld wrote:
> > Can userspace use the memory for something else if it's not passed to
> > getrandom?
> 
> I suspect the documentation answer here is, "no", even if technically it
> might happen to work on this kernel or that kernel. I suppose this could
> even be quasi-enforced by xoring the top bits with some vdso
> compile-time constant, so you can't rely on being able to dereference
> it yourself.
> [...]
> Then they're caught holding the bag? This doesn't seem much different
> from userspace shooting themselves in general, like writing garbage into
> the allocated states and then trying to use them. If this is something
> you really, really are concerned about, then maybe my cheesy dumb xor
> thing mentioned above would be a low effort mitigation here.

I implemented a sample of this, below. I think this is a bit silly,
though, and making this fully robust could take some effort. Overall, I
don't think we should do this.

However, the more I think about the args thing from the last email,
the more I like *that* idea. So I think I'll roll with that.

But this cheesy pointer obfuscation thing here, meh. But here's what it
could look like anyway:

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 2aaeb48d11be..7aff45165ce5 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -228,7 +228,7 @@ SYSCALL_DEFINE2(vgetrandom_alloc, struct vgetrandom_alloc_args __user *, uargs,
 	if (args.flags & VGRA_DEALLOCATE) {
 		if (args.size_per_each != state_size || args.num > max_states || !args.states)
 			return -EINVAL;
-		return vm_munmap(args.states, args.num * state_size);
+		return vm_munmap(args.states ^ VGETRANDOM_STATE_HI_TAINT, args.num * state_size);
 	}

 	/* These don't make sense as input values if allocating, so reject them. */
@@ -249,7 +249,7 @@ SYSCALL_DEFINE2(vgetrandom_alloc, struct vgetrandom_alloc_args __user *, uargs,

 	args.num = num_states;
 	args.size_per_each = state_size;
-	args.states = pages_addr;
+	args.states = pages_addr ^ VGETRANDOM_STATE_HI_TAINT;

 	ret = -EFAULT;
 	if (copy_to_user(uargs, &args, sizeof(args)))
diff --git a/include/vdso/getrandom.h b/include/vdso/getrandom.h
index cb624799a8e7..9a6aaf4d99d4 100644
--- a/include/vdso/getrandom.h
+++ b/include/vdso/getrandom.h
@@ -8,6 +8,7 @@

 #include <crypto/chacha.h>
 #include <vdso/limits.h>
+#include <linux/version.h>

 /**
  * struct vgetrandom_state - State used by vDSO getrandom() and allocated by vgetrandom_alloc().
@@ -41,4 +42,10 @@ struct vgetrandom_state {
 	bool 			in_use;
 };

+/* Be annoying by changing frequently enough. */
+#define VGETRANDOM_STATE_HI_TAINT ((unsigned long)(((LINUX_VERSION_CODE >> 16) + \
+				    (LINUX_VERSION_CODE >>  8) + (LINUX_VERSION_CODE >>  0) + \
+				    __GNUC__ + __GNUC_MINOR__ + __GNUC_PATCHLEVEL__) \
+				   & 0xff) << (BITS_PER_LONG - 8))
+
 #endif /* _VDSO_GETRANDOM_H */
diff --git a/lib/vdso/getrandom.c b/lib/vdso/getrandom.c
index 9ca624756432..14cbd349186c 100644
--- a/lib/vdso/getrandom.c
+++ b/lib/vdso/getrandom.c
@@ -57,7 +57,7 @@ __cvdso_getrandom_data(const struct vdso_rng_data *rng_info, void *buffer, size_
 		       unsigned int flags, void *opaque_state)
 {
 	ssize_t ret = min_t(size_t, INT_MAX & PAGE_MASK /* = MAX_RW_COUNT */, len);
-	struct vgetrandom_state *state = opaque_state;
+	struct vgetrandom_state *state = (void *)((unsigned long)opaque_state ^ VGETRANDOM_STATE_HI_TAINT);
 	size_t batch_len, nblocks, orig_len = len;
 	unsigned long current_generation;
 	void *orig_buffer = buffer;