From: "Jason A. Donenfeld" Subject: Re: [kernel-hardening] [PATCH v4 08/13] cifs: use get_random_u32 for 32-bit lock random Date: Thu, 8 Jun 2017 02:31:23 +0200 Message-ID: References: <20170606174804.31124-1-Jason@zx2c4.com> <20170606174804.31124-9-Jason@zx2c4.com> <20170608002523.m3h3jin4poav5xy2@thunk.org> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" To: "Theodore Ts'o" , "Jason A. Donenfeld" , Linux Crypto Mailing List , LKML , kernel-hardening@lists.openwall.com, Greg Kroah-Hartman , David Miller , Eric Biggers , Steve French Return-path: In-Reply-To: <20170608002523.m3h3jin4poav5xy2@thunk.org> Sender: linux-kernel-owner@vger.kernel.org List-Id: linux-crypto.vger.kernel.org On Thu, Jun 8, 2017 at 2:25 AM, Theodore Ts'o 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.