| Message ID | 20200529200347.2464284-1-keescook@chromium.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | lkdtm: Various clean ups | expand |
On Fri, May 29, 2020 at 01:03:43PM -0700, Kees Cook wrote: > Hi Greg, > > Can you please apply these patches to your drivers/misc tree for LKDTM? > It's mostly a collection of fixes and improvements and tweaks to the > selftest integration. Friendly ping -- we're past -rc2 now. :) Thanks! -Kees > > Thanks! > > -Kees > > Kees Cook (4): > lkdtm: Avoid more compiler optimizations for bad writes > lkdtm/heap: Avoid edge and middle of slabs > selftests/lkdtm: Reset WARN_ONCE to avoid false negatives > lkdtm: Make arch-specific tests always available > > drivers/misc/lkdtm/bugs.c | 45 +++++++++++++------------ > drivers/misc/lkdtm/heap.c | 9 ++--- > drivers/misc/lkdtm/lkdtm.h | 2 -- > drivers/misc/lkdtm/perms.c | 22 ++++++++---- > drivers/misc/lkdtm/usercopy.c | 7 ++-- > tools/testing/selftests/lkdtm/run.sh | 6 ++++ > tools/testing/selftests/lkdtm/tests.txt | 1 + > 7 files changed, 56 insertions(+), 36 deletions(-) > > -- > 2.25.1 >
On 6/23/20 4:10 PM, Kees Cook wrote: > On Fri, May 29, 2020 at 01:03:43PM -0700, Kees Cook wrote: >> Hi Greg, >> >> Can you please apply these patches to your drivers/misc tree for LKDTM? >> It's mostly a collection of fixes and improvements and tweaks to the >> selftest integration. > > Friendly ping -- we're past -rc2 now. :) > > Thanks! > > -Kees > >> >> Thanks! >> >> -Kees >> >> Kees Cook (4): >> lkdtm: Avoid more compiler optimizations for bad writes >> lkdtm/heap: Avoid edge and middle of slabs >> selftests/lkdtm: Reset WARN_ONCE to avoid false negatives >> lkdtm: Make arch-specific tests always available >> >> drivers/misc/lkdtm/bugs.c | 45 +++++++++++++------------ >> drivers/misc/lkdtm/heap.c | 9 ++--- >> drivers/misc/lkdtm/lkdtm.h | 2 -- >> drivers/misc/lkdtm/perms.c | 22 ++++++++---- >> drivers/misc/lkdtm/usercopy.c | 7 ++-- >> tools/testing/selftests/lkdtm/run.sh | 6 ++++ >> tools/testing/selftests/lkdtm/tests.txt | 1 + >> 7 files changed, 56 insertions(+), 36 deletions(-) >> >> -- >> 2.25.1 >> > >> Regardless, it seems arch/x86/um/asm/desc.h is not needed any more? > True that, we can rip the file. Has anyone fixed the uml build errors? thanks.
On Tue, Jun 23, 2020 at 04:32:12PM -0700, Randy Dunlap wrote: > On 6/23/20 4:10 PM, Kees Cook wrote: > > On Fri, May 29, 2020 at 01:03:43PM -0700, Kees Cook wrote: > >> Hi Greg, > >> > >> Can you please apply these patches to your drivers/misc tree for LKDTM? > >> It's mostly a collection of fixes and improvements and tweaks to the > >> selftest integration. > > > > Friendly ping -- we're past -rc2 now. :) > > > > Thanks! > > > > -Kees > > > >> > >> Thanks! > >> > >> -Kees > >> > >> Kees Cook (4): > >> lkdtm: Avoid more compiler optimizations for bad writes > >> lkdtm/heap: Avoid edge and middle of slabs > >> selftests/lkdtm: Reset WARN_ONCE to avoid false negatives > >> lkdtm: Make arch-specific tests always available > >> > >> drivers/misc/lkdtm/bugs.c | 45 +++++++++++++------------ > >> drivers/misc/lkdtm/heap.c | 9 ++--- > >> drivers/misc/lkdtm/lkdtm.h | 2 -- > >> drivers/misc/lkdtm/perms.c | 22 ++++++++---- > >> drivers/misc/lkdtm/usercopy.c | 7 ++-- > >> tools/testing/selftests/lkdtm/run.sh | 6 ++++ > >> tools/testing/selftests/lkdtm/tests.txt | 1 + > >> 7 files changed, 56 insertions(+), 36 deletions(-) > >> > >> -- > >> 2.25.1 > >> > > > > >> Regardless, it seems arch/x86/um/asm/desc.h is not needed any more? > > > True that, we can rip the file. > > Has anyone fixed the uml build errors? Oh, I thought that had nothing to do with the lkdtm changes? Should I rip out that file and resend?
On Tue, Jun 23, 2020 at 04:10:23PM -0700, Kees Cook wrote: > On Fri, May 29, 2020 at 01:03:43PM -0700, Kees Cook wrote: > > Hi Greg, > > > > Can you please apply these patches to your drivers/misc tree for LKDTM? > > It's mostly a collection of fixes and improvements and tweaks to the > > selftest integration. > > Friendly ping -- we're past -rc2 now. :) $ mdfrm -c ~/mail/todo/ 1970 messages in /home/gregkh/mail/todo/ You are in good company, give me some time to catch up please... thanks, greg k-h
----- Ursprüngliche Mail ----- >>> Regardless, it seems arch/x86/um/asm/desc.h is not needed any more? > >> True that, we can rip the file. > > Has anyone fixed the uml build errors? I didn't realize that this is a super urgent issue. ;-) Kees, if you want you can carry a patch in your series, I'll ack it. Otherwise I can also do a patch and bring it via the uml tree upstream as soon more fixes queued up. Thanks, //richard
On Wed, Jun 24, 2020 at 09:23:25AM +0200, Richard Weinberger wrote: > ----- Ursprüngliche Mail ----- > >>> Regardless, it seems arch/x86/um/asm/desc.h is not needed any more? > > > >> True that, we can rip the file. > > > > Has anyone fixed the uml build errors? > > I didn't realize that this is a super urgent issue. ;-) > > Kees, if you want you can carry a patch in your series, I'll ack it. > Otherwise I can also do a patch and bring it via the uml tree upstream > as soon more fixes queued up. I think the lkdtm change will tweak this bug, so I'm happy to carry the patch (I just haven't had time to create and test one). Is it really just as simple as removing arch/x86/um/asm/desc.h?
On 6/24/20 1:36 PM, Kees Cook wrote: > On Wed, Jun 24, 2020 at 09:23:25AM +0200, Richard Weinberger wrote: >> ----- Ursprüngliche Mail ----- >>>>> Regardless, it seems arch/x86/um/asm/desc.h is not needed any more? >>> >>>> True that, we can rip the file. >>> >>> Has anyone fixed the uml build errors? >> >> I didn't realize that this is a super urgent issue. ;-) >> >> Kees, if you want you can carry a patch in your series, I'll ack it. >> Otherwise I can also do a patch and bring it via the uml tree upstream >> as soon more fixes queued up. > > I think the lkdtm change will tweak this bug, so I'm happy to carry the > patch (I just haven't had time to create and test one). Is it really > just as simple as removing arch/x86/um/asm/desc.h? > I just tried that and the build is still failing, so No, it's not that simple. But thanks for offering. I'll just ignore the UML build errors for now.
On Wed, Jun 24, 2020 at 11:29 PM Randy Dunlap <rdunlap@infradead.org> wrote: > > On 6/24/20 1:36 PM, Kees Cook wrote: > > On Wed, Jun 24, 2020 at 09:23:25AM +0200, Richard Weinberger wrote: > >> ----- Ursprüngliche Mail ----- > >>>>> Regardless, it seems arch/x86/um/asm/desc.h is not needed any more? > >>> > >>>> True that, we can rip the file. > >>> > >>> Has anyone fixed the uml build errors? > >> > >> I didn't realize that this is a super urgent issue. ;-) > >> > >> Kees, if you want you can carry a patch in your series, I'll ack it. > >> Otherwise I can also do a patch and bring it via the uml tree upstream > >> as soon more fixes queued up. > > > > I think the lkdtm change will tweak this bug, so I'm happy to carry the > > patch (I just haven't had time to create and test one). Is it really > > just as simple as removing arch/x86/um/asm/desc.h? > > > > I just tried that and the build is still failing, so No, it's not that simple. > > But thanks for offering. > > I'll just ignore the UML build errors for now. This is a allyesconfig? I just gave CONFIG_LKDTM=y a try, builds fine here. But the desc.h in uml is still in vain and can be deleted AFAICT.
On 6/24/20 3:01 PM, Richard Weinberger wrote: > On Wed, Jun 24, 2020 at 11:29 PM Randy Dunlap <rdunlap@infradead.org> wrote: >> >> On 6/24/20 1:36 PM, Kees Cook wrote: >>> On Wed, Jun 24, 2020 at 09:23:25AM +0200, Richard Weinberger wrote: >>>> ----- Ursprüngliche Mail ----- >>>>>>> Regardless, it seems arch/x86/um/asm/desc.h is not needed any more? >>>>> >>>>>> True that, we can rip the file. >>>>> >>>>> Has anyone fixed the uml build errors? >>>> >>>> I didn't realize that this is a super urgent issue. ;-) >>>> >>>> Kees, if you want you can carry a patch in your series, I'll ack it. >>>> Otherwise I can also do a patch and bring it via the uml tree upstream >>>> as soon more fixes queued up. >>> >>> I think the lkdtm change will tweak this bug, so I'm happy to carry the >>> patch (I just haven't had time to create and test one). Is it really >>> just as simple as removing arch/x86/um/asm/desc.h? >>> >> >> I just tried that and the build is still failing, so No, it's not that simple. >> >> But thanks for offering. >> >> I'll just ignore the UML build errors for now. > > This is a allyesconfig? > I just gave CONFIG_LKDTM=y a try, builds fine here. > I'm building linux-next and it fails. > But the desc.h in uml is still in vain and can be deleted AFAICT.
On 6/24/20 3:23 PM, Randy Dunlap wrote: > On 6/24/20 3:01 PM, Richard Weinberger wrote: >> On Wed, Jun 24, 2020 at 11:29 PM Randy Dunlap <rdunlap@infradead.org> wrote: >>> >>> On 6/24/20 1:36 PM, Kees Cook wrote: >>>> On Wed, Jun 24, 2020 at 09:23:25AM +0200, Richard Weinberger wrote: >>>>> ----- Ursprüngliche Mail ----- >>>>>>>> Regardless, it seems arch/x86/um/asm/desc.h is not needed any more? >>>>>> >>>>>>> True that, we can rip the file. >>>>>> >>>>>> Has anyone fixed the uml build errors? >>>>> >>>>> I didn't realize that this is a super urgent issue. ;-) >>>>> >>>>> Kees, if you want you can carry a patch in your series, I'll ack it. >>>>> Otherwise I can also do a patch and bring it via the uml tree upstream >>>>> as soon more fixes queued up. >>>> >>>> I think the lkdtm change will tweak this bug, so I'm happy to carry the >>>> patch (I just haven't had time to create and test one). Is it really >>>> just as simple as removing arch/x86/um/asm/desc.h? >>>> >>> >>> I just tried that and the build is still failing, so No, it's not that simple. >>> >>> But thanks for offering. >>> >>> I'll just ignore the UML build errors for now. >> >> This is a allyesconfig? >> I just gave CONFIG_LKDTM=y a try, builds fine here. >> > > I'm building linux-next and it fails. More specifically, uml for i386 fails. x86_64 is OK. The problem is with the <asm/desc.h> file. I'm tampering with it... >> But the desc.h in uml is still in vain and can be deleted AFAICT.
On 6/24/20 3:35 PM, Randy Dunlap wrote: > On 6/24/20 3:23 PM, Randy Dunlap wrote: >> On 6/24/20 3:01 PM, Richard Weinberger wrote: >>> On Wed, Jun 24, 2020 at 11:29 PM Randy Dunlap <rdunlap@infradead.org> wrote: >>>> >>>> On 6/24/20 1:36 PM, Kees Cook wrote: >>>>> On Wed, Jun 24, 2020 at 09:23:25AM +0200, Richard Weinberger wrote: >>>>>> ----- Ursprüngliche Mail ----- >>>>>>>>> Regardless, it seems arch/x86/um/asm/desc.h is not needed any more? >>>>>>> >>>>>>>> True that, we can rip the file. >>>>>>> >>>>>>> Has anyone fixed the uml build errors? >>>>>> >>>>>> I didn't realize that this is a super urgent issue. ;-) >>>>>> >>>>>> Kees, if you want you can carry a patch in your series, I'll ack it. >>>>>> Otherwise I can also do a patch and bring it via the uml tree upstream >>>>>> as soon more fixes queued up. >>>>> >>>>> I think the lkdtm change will tweak this bug, so I'm happy to carry the >>>>> patch (I just haven't had time to create and test one). Is it really >>>>> just as simple as removing arch/x86/um/asm/desc.h? >>>>> >>>> >>>> I just tried that and the build is still failing, so No, it's not that simple. >>>> >>>> But thanks for offering. >>>> >>>> I'll just ignore the UML build errors for now. >>> >>> This is a allyesconfig? >>> I just gave CONFIG_LKDTM=y a try, builds fine here. >>> >> >> I'm building linux-next and it fails. > > More specifically, uml for i386 fails. x86_64 is OK. > The problem is with the <asm/desc.h> file. > I'm tampering with it... I'm not getting anywhere with this. Too many mazes of tiny twisty passages. >>> But the desc.h in uml is still in vain and can be deleted AFAICT. Looks like lkdtm/bugs.c needs to get/use arch/x86/include/asm/processor.h but it actually uses arch/x86/um/asm/processor*.h, which does not have the needed structs etc. Here are the build errors and warnings that I am seeing with allmodconfig: CC [M] drivers/misc/lkdtm/bugs.o In file included from ../arch/x86/include/asm/desc.h:11:0, from ../drivers/misc/lkdtm/bugs.c:17: ../arch/x86/include/asm/cpu_entry_area.h:65:42: error: invalid application of ‘sizeof’ to incomplete type ‘struct x86_hw_tss’ unsigned long stack[(PAGE_SIZE - sizeof(struct x86_hw_tss)) / sizeof(unsigned long)]; ^~~~~~ ../arch/x86/include/asm/cpu_entry_area.h:66:20: error: field ‘tss’ has incomplete type struct x86_hw_tss tss; ^~~ ../arch/x86/include/asm/cpu_entry_area.h:89:26: error: field ‘entry_stack_page’ has incomplete type struct entry_stack_page entry_stack_page; ^~~~~~~~~~~~~~~~ ../arch/x86/include/asm/cpu_entry_area.h:100:20: error: field ‘tss’ has incomplete type struct tss_struct tss; ^~~ In file included from ../drivers/misc/lkdtm/bugs.c:17:0: ../arch/x86/include/asm/desc.h:45:25: error: ‘GDT_ENTRIES’ undeclared here (not in a function); did you mean ‘LDT_ENTRIES’? struct desc_struct gdt[GDT_ENTRIES]; ^~~~~~~~~~~ LDT_ENTRIES ../arch/x86/include/asm/desc.h: In function ‘__set_tss_desc’: ../arch/x86/include/asm/desc.h:186:10: error: ‘__KERNEL_TSS_LIMIT’ undeclared (first use in this function); did you mean ‘__KERNEL__’? __KERNEL_TSS_LIMIT); ^~~~~~~~~~~~~~~~~~ __KERNEL__ ../arch/x86/include/asm/desc.h:186:10: note: each undeclared identifier is reported only once for each function it appears in ../arch/x86/include/asm/desc.h: In function ‘native_set_ldt’: ../arch/x86/include/asm/desc.h:202:40: error: ‘GDT_ENTRY_LDT’ undeclared (first use in this function); did you mean ‘GDT_ENTRY_INIT’? write_gdt_entry(get_cpu_gdt_rw(cpu), GDT_ENTRY_LDT, ^ ../arch/x86/include/asm/desc.h:123:75: note: in definition of macro ‘write_gdt_entry’ #define write_gdt_entry(dt, entry, desc, type) native_write_gdt_entry(dt, entry, desc, type) ^~~~~ ../arch/x86/include/asm/desc.h: In function ‘native_load_tr_desc’: ../arch/x86/include/asm/desc.h:259:31: error: ‘GDT_ENTRY_TSS’ undeclared (first use in this function); did you mean ‘GDT_ENTRIES’? asm volatile("ltr %w0"::"q" (GDT_ENTRY_TSS*8)); ^~~~~~~~~~~~~ GDT_ENTRIES ../arch/x86/include/asm/desc.h: In function ‘native_load_tls’: ../arch/x86/include/asm/desc.h:278:33: error: ‘struct thread_struct’ has no member named ‘tls_array’ gdt[GDT_ENTRY_TLS_MIN + i] = t->tls_array[i]; ^~ In file included from ../arch/x86/include/asm/string.h:3:0, from ../include/linux/string.h:20, from ../arch/x86/um/asm/processor_32.h:9, from ../arch/x86/um/asm/processor.h:10, from ../include/linux/rcupdate.h:30, from ../include/linux/rculist.h:11, from ../include/linux/pid.h:5, from ../include/linux/sched.h:14, from ../drivers/misc/lkdtm/bugs.c:10: ../arch/x86/include/asm/desc.h: In function ‘force_reload_TR’: ../arch/x86/include/asm/desc.h:288:18: error: ‘GDT_ENTRY_TSS’ undeclared (first use in this function); did you mean ‘GDT_ENTRIES’? memcpy(&tss, &d[GDT_ENTRY_TSS], sizeof(tss_desc)); ^ ../arch/x86/include/asm/string_32.h:182:45: note: in definition of macro ‘memcpy’ #define memcpy(t, f, n) __builtin_memcpy(t, f, n) ^ In file included from ../include/linux/kernel.h:11:0, from ../drivers/misc/lkdtm/lkdtm.h:7, from ../drivers/misc/lkdtm/bugs.c:8: ../arch/x86/include/asm/desc.h: In function ‘invalidate_tss_limit’: ../arch/x86/include/asm/desc.h:327:32: error: ‘TIF_IO_BITMAP’ undeclared (first use in this function); did you mean ‘CONFIG_SBITMAP’? if (unlikely(test_thread_flag(TIF_IO_BITMAP))) ^ ../include/linux/compiler.h:78:42: note: in definition of macro ‘unlikely’ # define unlikely(x) __builtin_expect(!!(x), 0) ^ ../arch/x86/include/asm/desc.h:327:15: note: in expansion of macro ‘test_thread_flag’ if (unlikely(test_thread_flag(TIF_IO_BITMAP))) ^~~~~~~~~~~~~~~~ In file included from ../drivers/misc/lkdtm/bugs.c:17:0: ../arch/x86/include/asm/desc.h: At top level: ../arch/x86/include/asm/desc.h:334:0: warning: "LDT_empty" redefined #define LDT_empty(info) \ In file included from ../arch/um/include/asm/mmu.h:10:0, from ../include/linux/mm_types.h:18, from ../include/linux/sched/signal.h:13, from ../drivers/misc/lkdtm/bugs.c:11: ../arch/x86/um/asm/mm_context.h:65:0: note: this is the location of the previous definition #define LDT_empty(info) (_LDT_empty(info)) In file included from ../drivers/misc/lkdtm/bugs.c:17:0: ../arch/x86/include/asm/desc.h: In function ‘get_cpu_gdt_rw’: ../arch/x86/include/asm/desc.h:54:1: warning: control reaches end of non-void function [-Wreturn-type] } ^
On Wed, Jun 24, 2020 at 06:45:47PM -0700, Randy Dunlap wrote: > Looks like lkdtm/bugs.c needs to get/use arch/x86/include/asm/processor.h > but it actually uses arch/x86/um/asm/processor*.h, which does not have the > needed structs etc. Should I just test for !UML in bugs.c? (This is all for the lkdtm_DOUBLE_FAULT() test.) I already do those kinds of checks for the lkdtm_UNSET_SMEP() test. e.g.: diff --git a/drivers/misc/lkdtm/bugs.c b/drivers/misc/lkdtm/bugs.c index 736675f0a246..f3e7040a7739 100644 --- a/drivers/misc/lkdtm/bugs.c +++ b/drivers/misc/lkdtm/bugs.c @@ -13,7 +13,7 @@ #include <linux/uaccess.h> #include <linux/slab.h> -#ifdef CONFIG_X86_32 +#if IS_ENABLED(CONFIG_X86_32) && !IS_ENABLED(CONFIG_UML) #include <asm/desc.h> #endif @@ -419,7 +419,7 @@ void lkdtm_UNSET_SMEP(void) void lkdtm_DOUBLE_FAULT(void) { -#ifdef CONFIG_X86_32 +#if IS_ENABLED(CONFIG_X86_32) && !IS_ENABLED(CONFIG_UML) /* * Trigger #DF by setting the stack limit to zero. This clobbers * a GDT TLS slot, which is okay because the current task will die
----- Ursprüngliche Mail ----- > Von: "Kees Cook" <keescook@chromium.org> > An: "Randy Dunlap" <rdunlap@infradead.org> > CC: "Richard Weinberger" <richard.weinberger@gmail.com>, "richard" <richard@nod.at>, "Greg Kroah-Hartman" > <gregkh@linuxfoundation.org>, "Prasad Sodagudi" <psodagud@codeaurora.org>, "Sami Tolvanen" <samitolvanen@google.com>, > "Amit Daniel Kachhap" <amit.kachhap@arm.com>, "linux-kselftest" <linux-kselftest@vger.kernel.org>, "clang-built-linux" > <clang-built-linux@googlegroups.com>, "linux-kernel" <linux-kernel@vger.kernel.org> > Gesendet: Donnerstag, 25. Juni 2020 08:06:18 > Betreff: Re: [PATCH drivers/misc 0/4] lkdtm: Various clean ups > On Wed, Jun 24, 2020 at 06:45:47PM -0700, Randy Dunlap wrote: >> Looks like lkdtm/bugs.c needs to get/use arch/x86/include/asm/processor.h >> but it actually uses arch/x86/um/asm/processor*.h, which does not have the >> needed structs etc. > > Should I just test for !UML in bugs.c? (This is all for the > lkdtm_DOUBLE_FAULT() test.) I already do those kinds of checks for the > lkdtm_UNSET_SMEP() test. e.g.: Just had a look. Yes, this sounds good to me. UML has CONFIG_X86_32=y but no GDT. :-) Thanks, //richard
On 6/24/20 11:24 PM, Richard Weinberger wrote: > ----- Ursprüngliche Mail ----- >> Von: "Kees Cook" <keescook@chromium.org> >> An: "Randy Dunlap" <rdunlap@infradead.org> >> CC: "Richard Weinberger" <richard.weinberger@gmail.com>, "richard" <richard@nod.at>, "Greg Kroah-Hartman" >> <gregkh@linuxfoundation.org>, "Prasad Sodagudi" <psodagud@codeaurora.org>, "Sami Tolvanen" <samitolvanen@google.com>, >> "Amit Daniel Kachhap" <amit.kachhap@arm.com>, "linux-kselftest" <linux-kselftest@vger.kernel.org>, "clang-built-linux" >> <clang-built-linux@googlegroups.com>, "linux-kernel" <linux-kernel@vger.kernel.org> >> Gesendet: Donnerstag, 25. Juni 2020 08:06:18 >> Betreff: Re: [PATCH drivers/misc 0/4] lkdtm: Various clean ups > >> On Wed, Jun 24, 2020 at 06:45:47PM -0700, Randy Dunlap wrote: >>> Looks like lkdtm/bugs.c needs to get/use arch/x86/include/asm/processor.h >>> but it actually uses arch/x86/um/asm/processor*.h, which does not have the >>> needed structs etc. >> >> Should I just test for !UML in bugs.c? (This is all for the >> lkdtm_DOUBLE_FAULT() test.) I already do those kinds of checks for the >> lkdtm_UNSET_SMEP() test. e.g.: > > Just had a look. Yes, this sounds good to me. UML has CONFIG_X86_32=y but no GDT. :-) Sounds good to me also. Thanks.