Messages in this thread | | | Date | Thu, 2 Sep 2021 10:35:43 -0700 | From | Kees Cook <> | Subject | Re: [PATCH 0/2]: ARM: Enable THREAD_INFO_IN_TASK |
| |
On Thu, Sep 02, 2021 at 05:53:53PM +0100, Russell King (Oracle) wrote: > On Thu, Sep 02, 2021 at 08:54:26AM -0700, Keith Packard wrote: > > Placing thread_info in the kernel stack leaves it vulnerable to stack > > overflow attacks. This short series addresses that by using the > > existing THREAD_INFO_IN_TASK infrastructure. > > > > As this is my first patch in this part of the kernel, I'm looking for > > feedback about the general approach as well as specific comments on > > places where I've missed something. > > > > I've only run this on armhf running under qemu, so while I've tried to > > make patches for other code paths, I haven't been able to test those. > > > > (yes, I know checkpatch.pl complains about whitespace in asm-offsets.c, I > > decided to leave the existing whitespace alone) > > > > Signed-off-by: Keith Packard <keithpac@amazon.com> > > I think you're introducing a circular dependency with this for > certain kernel configurations: > > E.g. Have you tried running this with CONFIG_CPU_V6 enabled? > > +#define raw_smp_processor_id() this_cpu_read(cpu_number) > +#define __smp_processor_id() __this_cpu_read(cpu_number) > + > +DECLARE_PER_CPU_READ_MOSTLY(unsigned int, cpu_number); > > this_cpu_read() is defined as: > > #define this_cpu_read(pcp) __pcpu_size_call_return(this_cpu_read_, pcp) > (which will call this_cpu_read_4) > > #define this_cpu_read_4(pcp) this_cpu_generic_read(pcp) > => __this_cpu_generic_read_nopreempt() > => ___ret = READ_ONCE(*raw_cpu_ptr(&(pcp))); > > #define raw_cpu_ptr(ptr) \ > ({ \ > __verify_pcpu_ptr(ptr); \ > arch_raw_cpu_ptr(ptr); \ > }) > > #ifndef arch_raw_cpu_ptr > #define arch_raw_cpu_ptr(ptr) SHIFT_PERCPU_PTR(ptr, __my_cpu_offset) > #endif > > #ifndef __my_cpu_offset > #define __my_cpu_offset per_cpu_offset(raw_smp_processor_id()) > #endif > > ... which then leads back to a use of raw_smp_processor_id(), thereby > creating a circular loop of preprocessor definitions that the compiler > can't resolve.
If this isn't easy to fix, perhaps this can be a V7-only feature?
-- Kees Cook
| |