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;
