Hi Greg, On Thu, Jan 05, 2023 at 03:38:30PM +0100, Greg KH wrote: > On Thu, Jan 05, 2023 at 11:11:15AM -0300, Raphael S. Carvalho wrote: > > On Thu, Jan 5, 2023 at 10:42 AM Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote: > > > > > > On Thu, Jan 05, 2023 at 08:35:05PM +0800, Aaron Lu wrote: > > > > To capture potential programming errors like mistakenly setting Global > > > > bit on kernel page table entries, a selftest for meltdown is added. > > > > > > > > This selftest is based on Pavel Boldin's work at: > > > > https://github.com/linux-test-project/ltp/blob/master/testcases/cve/meltdown.c > > > > > > > > In addition to the existing test of reading kernel variable > > > > saved_command_line from user space, one more test of reading user local > > > > variable through kernel direct map address is added. For the existing > > > > test(reading saved_command_line) to report a failure, both the high kernel > > > > mapping and low kernel mapping have to be in leaked state; For the added > > > > test(read local var), only low kernel mapping leak is enough to trigger > > > > a test fail, so both tests are useful. > > > > > > > > Test results of 10 runs: > > > > > > > > On v6.1-rc8 with nopti kernel cmdline option: > > > > > > > > host test_out_rate_1 test_out_rate_2 > > > > lkp-bdw-de1 50% 100% > > > > lkp-hsw-d01 70% 100% > > > > lkp-hsw-d02 0% 80% > > > > lkp-hsw-d03 60% 100% > > > > lkp-hsw-d04 20% 100% > > > > lkp-hsw-d05 60% 100% > > > > lkp-ivb-d01 0% 70% > > > > lkp-kbl-d01 100% 100% > > > > lkp-skl-d02 100% 90% > > > > lkp-skl-d03 90% 100% > > > > lkp-skl-d05 60% 100% > > > > kbl-vm 100% 80% > > > > 2 other machines have 0% rate for both tests. > > > > > > > > bdw=broadwell, hsw=haswell, ivb=ivybridge, etc. > > > > > > > > test_out_rate_1: test reports fail rate for the test of reading > > > > saved_command_line from user space; > > > > test_out_rate_2: test reports fail rate for the test of reading user > > > > local variable through kernel direct map address in user space. > > > > > > > > On v5.19 without nopti cmdline option: > > > > host test_out_rate_2 > > > > lkp-bdw-de1 80% > > > > lkp-hsw-4ex1 50% > > > > lkp-hsw-d01 30% > > > > lkp-hsw-d03 10% > > > > lkp-hsw-d04 10% > > > > lkp-kbl-d01 10% > > > > kbl-vm 80% > > > > 7 other machines have 0% rate for test2. > > > > > > > > Also tested on an i386 VM with 512M memory and the test out rate is 100% > > > > when adding nopti to kernel cmdline with v6.1-rc8. > > > > > > > > Main changes I made from Pavel Boldin's meltdown test are: > > > > - Replace rdtscll() and clflush() with kernel's implementation; > > > > - Reimplement find_symbol_in_file() to avoid bringing in LTP's library > > > > functions; > > > > - Coding style changes: placing the function return type in the same > > > > line of the function. > > > > > > > > Signed-off-by: Aaron Lu <aaron.lu@xxxxxxxxx> > > > > --- > > > > Notable changes from RFC v3: > > > > - Drop RFC tag; > > > > - Change the base code from zlib licensed one to GPL licensed one. > > > > > > Sorry, but this still gets my NAK for the issues raised in previous > > > reviews that are not addressed here for some reason :( > > > > Greg, the selftest is no longer based on > > https://github.com/IAIK/meltdown/blob/master/LICENSE, which is > > originally zlib licensed. In this version, Aaron is basing the test on > > https://github.com/linux-test-project/ltp/blob/master/testcases/cve/meltdown.c, > > which is indeed licensed with: GPL-2.0-or-later > > That's not what the submission looks like, it looks a lot like the last > one from the first defines and variables... > > But hey, what do I know, I'm not a lawyer which is why I keep insisting > that one from Intel actually read over this submission and sign-off on > it to verify that they agree with all of this. As Raphael has kindly clarified for me, I'm now taking GPL-2.0-or-later licensed code and did some modifications and then released it as GPL-2.0-or-later, I suppose this is legally a right thing to do for anyone? If you do not trust what I've done is what I've claimed, now the original author Pavel Boldin has given the patch a "LGTM" tag, does that address your concern? Thanks, Aaron
