On Thu, Jun 8, 2017 at 2:25 AM, Theodore Ts'o <tytso@xxxxxxx> 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.
