linux-kernel - Re: [PATCH 1/1] checkpatch: recognise NOKPROBE_SYMBOL for blank line detection after function/struct/union/enum

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: <86bb384b-0084-05ac-3a13-ef462d1c0a35@linux.ibm.com>
Date:   Tue, 24 Jan 2023 13:16:01 +0100
From:   Christian Borntraeger <borntraeger@...ux.ibm.com>
To:     Joe Perches <joe@...ches.com>, Andy Whitcroft <apw@...onical.com>
Cc:     Dwaipayan Ray <dwaipayanray1@...il.com>,
        Lukas Bulwahn <lukas.bulwahn@...il.com>,
        linux-kernel@...r.kernel.org,
        kernel-janitors <kernel-janitors@...r.kernel.org>
Subject: Re: [PATCH 1/1] checkpatch: recognise NOKPROBE_SYMBOL for blank line
 detection after function/struct/union/enum



Am 20.01.23 um 17:10 schrieb Joe Perches:
> On Fri, 2023-01-20 at 12:46 +0100, Christian Borntraeger wrote:
>> NOKPROBE_SYMBOL should be treated like EXPORT_SYMBOL to avoid
>> false positives.
> []
>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> []
>> @@ -3995,6 +3995,7 @@ sub process {
>>   		    $line =~ /^\+/ &&
>>   		    !($line =~ /^\+\s*$/ ||
>>   		      $line =~ /^\+\s*(?:EXPORT_SYMBOL|early_param)/ ||
>> +		      $line =~ /^\+\s*NOKPROBE_SYMBOL/ ||
>>   		      $line =~ /^\+\s*MODULE_/i ||
>>   		      $line =~ /^\+\s*\#\s*(?:end|elif|else)/ ||
>>   		      $line =~ /^\+[a-z_]*init/ ||
> 
> $ git grep -P -w -oh '\w*NOKPROBE_SYMBOL\w*' | sort | uniq -c
>       60 _ASM_NOKPROBE_SYMBOL
>        2 __NOKPROBE_SYMBOL
>      304 NOKPROBE_SYMBOL
> 
> Any issue with these other variants? (it doesn't seem so)
7
The other variants should not matter.
> 
> btw: __NOKPROBE_SYMBOL has a ; at the end of its definition.
> 
> include/asm-generic/kprobes.h:# define __NOKPROBE_SYMBOL(fname)                         \
> include/asm-generic/kprobes.h-static unsigned long __used                                       \
> include/asm-generic/kprobes.h-  __section("_kprobe_blacklist")                          \
> include/asm-generic/kprobes.h-  _kbl_addr_##fname = (unsigned long)fname;
> 
> And so there are extra ; uses after ever use of NOKPROBE_SYMBOL(foo);
> 
> $ git grep -w NOKPROBE_SYMBOL | grep ';' | wc -l
> 285
> 
> There are only 10 uses of NOKPROBE_SYMBOL(foo) without a semicolon
> 
> $ git grep -P '\bNOKPROBE_SYMBOL\s*\(\s*\w+\s*\)\s*(?:[^\;]|$)'
> arch/arm/kernel/traps.c:NOKPROBE_SYMBOL(do_undefinstr)
> arch/arm/probes/kprobes/opt-arm.c:NOKPROBE_SYMBOL(optimized_callback)
> arch/arm64/kernel/kgdb.c:NOKPROBE_SYMBOL(kgdb_brk_fn)
> arch/powerpc/mm/cacheflush.c:NOKPROBE_SYMBOL(flush_dcache_icache_phys)
> arch/powerpc/platforms/82xx/pq2.c:NOKPROBE_SYMBOL(pq2_restart)
> include/asm-generic/kprobes.h:# define NOKPROBE_SYMBOL(fname)   __NOKPROBE_SYMBOL(fname)
> include/asm-generic/kprobes.h:# define NOKPROBE_SYMBOL(fname)
> kernel/fail_function.c:NOKPROBE_SYMBOL(fei_kprobe_handler)
> kernel/kprobes.c:NOKPROBE_SYMBOL(__kretprobe_trampoline_handler)
> kernel/trace/trace_eprobe.c:NOKPROBE_SYMBOL(process_fetch_insn)
> kernel/trace/trace_kprobe.c:NOKPROBE_SYMBOL(process_fetch_insn)
> kernel/trace/trace_uprobe.c:NOKPROBE_SYMBOL(process_fetch_insn)
> 
> The actual uses with NOKPROBE_SYMBOL(foo) should likely have semicolons added
> and the __NOKPROBE_SYMBOL definition should have the semicolon removed.

That would be an independent check I guess.