linux-kernel - Re: [RFC PATCH v1 2/3] powerpc: Mark all .S files invalid for objtool

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]

Date:   Tue, 20 Jun 2023 12:10:23 +0530
From:   Naveen N Rao <naveen@...nel.org>
To:     Christophe Leroy <christophe.leroy@...roup.eu>,
        Josh Poimboeuf <jpoimboe@...nel.org>,
        Michael Ellerman <mpe@...erman.id.au>,
        Nicholas Piggin <npiggin@...il.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Sathvika Vasireddy <sv@...ux.ibm.com>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>
Subject: Re: [RFC PATCH v1 2/3] powerpc: Mark all .S files invalid for objtool

Christophe Leroy wrote:
> 
> 
> Le 20/06/2023 à 08:04, Naveen N Rao a écrit :
>> Christophe Leroy wrote:
>>> A lot of work is required in .S files in order to get them ready
>>> for objtool checks.
>>>
>>> For the time being, exclude them from the checks.
>>>
>>> This is done with the script below:
>>>
>>>     #!/bin/sh
>>>     DIRS=`find arch/powerpc -name "*.S" -exec dirname {} \; | sort | 
>>> uniq`
>>>     for d in $DIRS
>>>     do
>>>         pushd $d
>>>         echo >> Makefile
>>>         for f in *.S
>>>         do
>>>             echo "OBJECT_FILES_NON_STANDARD_$f := y" | sed s/"\.S"/".o"/g
>>>         done >> Makefile
>>>         popd
>>>     done
>>>
>>> Signed-off-by: Christophe Leroy <christophe.leroy@...roup.eu>
>>> ---
>>>  arch/powerpc/boot/Makefile                 | 17 +++++++++
>>>  arch/powerpc/crypto/Makefile               | 13 +++++++
>>>  arch/powerpc/kernel/Makefile               | 44 ++++++++++++++++++++++
>>>  arch/powerpc/kernel/trace/Makefile         |  4 ++
>>>  arch/powerpc/kernel/vdso/Makefile          | 11 ++++++
>>>  arch/powerpc/kexec/Makefile                |  2 +
>>>  arch/powerpc/kvm/Makefile                  | 13 +++++++
>>>  arch/powerpc/lib/Makefile                  | 25 ++++++++++++
>>>  arch/powerpc/mm/book3s32/Makefile          |  3 ++
>>>  arch/powerpc/mm/nohash/Makefile            |  3 ++
>>>  arch/powerpc/perf/Makefile                 |  2 +
>>>  arch/powerpc/platforms/44x/Makefile        |  2 +
>>>  arch/powerpc/platforms/52xx/Makefile       |  3 ++
>>>  arch/powerpc/platforms/83xx/Makefile       |  2 +
>>>  arch/powerpc/platforms/cell/spufs/Makefile |  3 ++
>>>  arch/powerpc/platforms/pasemi/Makefile     |  2 +
>>>  arch/powerpc/platforms/powermac/Makefile   |  3 ++
>>>  arch/powerpc/platforms/powernv/Makefile    |  3 ++
>>>  arch/powerpc/platforms/ps3/Makefile        |  2 +
>>>  arch/powerpc/platforms/pseries/Makefile    |  2 +
>>>  arch/powerpc/purgatory/Makefile            |  3 ++
>>>  arch/powerpc/sysdev/Makefile               |  3 ++
>>>  arch/powerpc/xmon/Makefile                 |  3 ++
>>>  23 files changed, 168 insertions(+)
>>>
>> 
>> I think it might be better to have a config option so that architectures 
>> can opt-in to skip objtool on asm files. We can then do:
> 
> Well, the idea here was to initially opt out every file in order to 
> quickly add support for objtool uaccess checking, and then opt-in back 
> files one by one once they are ready for it.

That was my initial thought too.

> 
> In most files,all we have to do is to replace all _GLOBAL() by 
> SYM_FUNC_START(), add a SYM_FUNC_END() at the end of the function, then 
> do the same with SYM_FUNC_START_LOCAL() with all non global functions.
> 
> That's easy to do and worth it but it takes time, hence the idea of an 
> incremental approach.

Right. But until that's done, I am not sure it is worth the churn to the 
Makefiles.

Besides, it isn't clear to me that the existing features we are 
targeting for objtool on powerpc need objtool to run on asm files (so, 
an incremental approach may not really get us much). Objtool --mcount 
doesn't care about .S files.  I suppose uaccess validation also doesn't 
need it. It's likely just stack validation (should we choose to enable 
it) that needs objtool to be run on asm files.

The first patch series converting much of our .S files can drop the 
config option and exclude the remaining files.


- Naveen