linux-kernel - Re: [PATCH 2/3] ARM: Move thread_info into task

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87zgsqdy2m.fsf@keithp.com>
Date:   Sun, 05 Sep 2021 23:20:49 -0700
From:   Keith Packard <keithp@...thp.com>
To:     Ard Biesheuvel <ardb@...nel.org>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Abbott Liu <liuwenliang@...wei.com>,
        Alexander Sverdlin <alexander.sverdlin@...il.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Anshuman Khandual <anshuman.khandual@....com>,
        Arnd Bergmann <arnd@...db.de>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Florian Fainelli <f.fainelli@...il.com>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Hartley Sweeten <hsweeten@...ionengravers.com>,
        Jens Axboe <axboe@...nel.dk>, Jian Cai <jiancai@...gle.com>,
        Joe Perches <joe@...ches.com>,
        Kees Cook <keescook@...omium.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski@...onical.com>,
        Linus Walleij <linus.walleij@...aro.org>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        Manivannan Sadhasivam <mani@...nel.org>,
        Marc Zyngier <maz@...nel.org>,
        Masahiro Yamada <masahiroy@...nel.org>,
        Miguel Ojeda <ojeda@...nel.org>,
        Mike Rapoport <rppt@...nel.org>,
        Nathan Chancellor <nathan@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Nicolas Pitre <nico@...xnic.net>,
        Rob Herring <robh@...nel.org>,
        Russell King <linux@...linux.org.uk>,
        Thomas Gleixner <tglx@...utronix.de>,
        Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>,
        Valentin Schneider <valentin.schneider@....com>,
        Viresh Kumar <viresh.kumar@...aro.org>,
        "Wolfram Sang (Renesas)" <wsa+renesas@...g-engineering.com>,
        YiFei Zhu <yifeifz2@...inois.edu>
Subject: Re: [PATCH 2/3] ARM: Move thread_info into task_struct (v7 only)

Ard Biesheuvel <ardb@...nel.org> writes:

(sorry, missed a couple of comments, and also neglected to thank you for
your review!)

>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index 24804f11302d..1c1ded500a2b 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -128,6 +128,7 @@ config ARM
>>         select RTC_LIB
>>         select SET_FS
>>         select SYS_SUPPORTS_APM_EMULATION
>> +       select THREAD_INFO_IN_TASK if CPU_V7
>
> CPU_V6K also supports this

I've only tested on V7; help getting testing for V6K so we could enable
that as well would be greatly appreciated.

>> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
>> index 415c3514573a..71a2ba4549d3 100644
>> --- a/arch/arm/Makefile
>> +++ b/arch/arm/Makefile
>> @@ -284,6 +284,14 @@ stack_protector_prepare: prepare0
>>         $(eval GCC_PLUGINS_CFLAGS += $(SSP_PLUGIN_CFLAGS))
>>  endif
>>
>> +ifdef CONFIG_SMP
>> +prepare: task_cpu_prepare
>> +
>> +PHONY += task_cpu_prepare
>> +task_cpu_prepare: prepare0
>> +       $(eval KBUILD_CFLAGS += -D_TSK_CPU=$(shell awk '{if ($$2 == "TSK_CPU") print $$3;}' include/generated/asm-offsets.h))
>> +endif
>> +
>>  all:   $(notdir $(KBUILD_IMAGE))
>>
>
> This is rather horrid, and removed again in the next patch. Can we
> omit it entirely?

Yeah, this did get rather ugly -- I wanted to use the task_struct in the
raw_smp_processor_id() macro, but discovered that I couldn't include
linux/sched.h in that file.

I found this lovely hack in the powerpc kernel code and just lifted it
from there. In terms of code changes, this one is "smaller" than
including the new per-cpu variable, "cpu_number", but in terms of
ugliness, it's hard to argue which is cleaner.

I was going for fewest logical changes in structure with this patch
instead of cleanest result to try and make it slightly easier to review.

Happy to squash these two patches together if you'd prefer; I initially
wrote the code with the cpu_number variable but then discovered that I
didn't need it if I used the cpu value in the task_struct...

-- 
-keith

Download attachment "signature.asc" of type "application/pgp-signature" (833 bytes)