On 2021-03-29 09:26, Jann Horn wrote:
Hi!
I'm currently in the middle of writing a blogpost on some Linux kernel
stuff; while working on that I looked at the randstruct version in
Linus' tree a bit and noticed two potential areas for improvement. I
have no idea whether any of this (still) applies to the PaX/grsecurity
version, but since the code originates from there, I figured I should
also send this to the original authors.
[...]
I don't know whether the amount of information leakage would be enough
to actually determine the seed - and I'm not a cryptographer, I have
no clue how much output from the RNG you'd actually need to recover
the seed (and an attacker would not even be getting raw RNG output,
but RNG output after additional modulo operations). But if the goal
here is to ensure that an attacker without access to the binary kernel
image can't determine struct layouts without a proper leak primitive,
even if they know exactly from which sources and with what
configuration the kernel was built, then I think this needs a
cryptographically secure RNG.
Hi,
I just wanted to add something that stood out to me (assuming I'm
reading the code correctly):
It looks like the per-struct seed is constructed only based on a hash of
the struct name (using name_hash()), and anonymous structs use the name
"anonymous", which means that anonymous structs with the same number of
members will always be shuffled the same way (using full_shuffle() at
least). Which means that you can gain information about one struct and
potentially use it on another. It doesn't look like anonymous structs
being randomized is very common, a quick run against kernel/fork.c shows
there's only 3 cases and they all have different numbers of members (7,
59, and 182). In any case, to mitigate this, maybe include some details
of the struct (original member offsets/sizes/names) in the per-struct
seed derivation?
I definitely second the recommendation to use cryptographically secure
algorithms -- specifically, use a 256-bit HMAC that depends on the seed
instead of name_hash() and a cryptographically secure PRNG for ranval().
Vegard
