On Thu, Jun 8, 2017 at 2:25 AM, Theodore Ts'o <ty...@mit.edu> wrote:
> There's a bigger problem here, which is that cifs_lock_secret is a
> 32-bit value which is being used to obscure flock->fl_owner before it
> is sent across the wire. But flock->fl_owner is a pointer to the
> struct file *, so 64-bit architecture, the high 64-bits of a kernel
> pointer is being exposed to anyone using tcpdump. (Oops, I'm showing
> my age; I guess all the cool kids are using Wireshark these days.)
> Worse, the obscuring is being done using XOR. How an active attacker
> might be able to trivially reverse engineer the 32-bit "secret" is
> left as an exercise to the reader. The bottom line is if the goal is
> to hide the memory location of a struct file from an attacker,
> cifs_lock_secret is about as useful as a TSA agent doing security
> theatre at an airport. Which is to say, it makes the civilians feel
> good. :-)
High five for taking the deep dive and actually reading how this all
works. Nice bug!
> Not waiting
> for the CRNG to be fully initialized is the *least* of its problems.
The kernel is vast and filled with tons of bugs of many sorts. On this
reasoning, maybe I should spend my time auditing web apps instead,
which are usually the "front door" of bugs? I like the puzzles of
random.c. I also had a real world need for wait_for_random_bytes() in
a module I'm writing.
But anyway, your general point is a really good one. Tons of callers
of the random functions are doing it wrong in one way or another.
Spending time looking at those is probably a good idea...
> Anyway, I'll include this commit in the dev branch of the random tree,
> since it's not going to make things worse.