On Wed, May 27, 2020 at 09:07:55AM +0200, Greg KH wrote:
On Tue, May 26, 2020 at 07:24:03PM +0200, Wojtek Porczyk wrote:
On Tue, May 26, 2020 at 06:32:35PM +0200, Greg KH wrote:
On Tue, May 26, 2020 at 08:48:35AM -0700, Andi Kleen wrote:
On Tue, May 26, 2020 at 08:56:18AM +0200, Greg KH wrote:
On Mon, May 25, 2020 at 10:28:48PM -0700, Andi Kleen wrote:
From: Andi Kleen ak@linux.intel.com
Since there seem to be kernel modules floating around that set FSGSBASE incorrectly, prevent this in the CR4 pinning. Currently CR4 pinning just checks that bits are set, this also checks that the FSGSBASE bit is not set, and if it is clears it again.
So we are trying to "protect" ourselves from broken out-of-tree kernel modules now?
Well it's a specific case where we know they're opening a root hole unintentionally. This is just an pragmatic attempt to protect the users in the short term.
Can't you just go and fix those out-of-tree kernel modules instead? What's keeping you all from just doing that instead of trying to force the kernel to play traffic cop?
We'd very much welcome any help really, but we're under impression that this couldn't be done correctly in a module, so this hack occured.
Really? How is this hack anything other than a "prevent a kernel module from doing something foolish" change?
By "this hack" I meant our module [1], which just enables FSGSBASE bit without doing everything else that Sasha's patchset does, and that "everything else" is there to prevent a gaping root hoole.
Original author wanted module for the reason snipped below, but Sasha's patchset can't be packaged into module. I'll be happy to be corrected on this point, I personally have next to no kernel programming experience.
This kernel change I think is correct, because if kernel assumes some invariants, it's a good idea to actually enforce them, isn't it? So we don't have anything against this kernel change. We'll have to live with it, with our hand forced.
Why can't you just change the kernel module's code to not do this? What prevents that from happening right now which would prevent the need to change a core api from being abused in such a way?
[snip]
I'm sorry, but I still do not understand. Your kernel module calls the core with this bit being set, and this new kernel patch is there to prevent the bit from being set and will WARN_ON() if it happens. Why can't you just change your module code to not set the bit?
Because we need userspace access to wrfsbase, which this bit enables:
https://github.com/oscarlab/graphene/blob/58c53ad747579225bf29e3506d883586ff... https://github.com/oscarlab/graphene/blob/0dd84ddf943d256e5494f07cb41b1d0ece... https://github.com/oscarlab/graphene/blob/e4e16aa10e3c2221170aee7da66370507c...
Do you have a pointer to the kernel module code that does this operation which this core kernel change will try to prevent from happening?
Sure: https://github.com/oscarlab/graphene-sgx-driver/blob/a73de5fed96fc330fc0417d...
The whole module is 86 sloc, and the sole purpose is to set this bit on load and clear on unload.
[1] ^