Messages in this thread | | | From | "Jason A. Donenfeld" <> | Date | Thu, 8 Jun 2017 02:34:40 +0200 | Subject | Re: [kernel-hardening] [PATCH v4 08/13] cifs: use get_random_u32 for 32-bit lock random |
| |
On Thu, Jun 8, 2017 at 2:25 AM, Theodore Ts'o <tytso@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.
Great, thanks.
| |