linux-kernel - Re: checkpatch.pl should suggest _

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 for Android: free password hash cracker in your pocket

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

Message-ID: <7c4db60a2b1976a92b5c824c7d24c4c77aa57278.camel@perches.com>
Date:   Fri, 09 Aug 2019 15:58:41 -0700
From:   Joe Perches <joe@...ches.com>
To:     Nick Desaulniers <ndesaulniers@...gle.com>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Sedat Dilek <sedat.dilek@...il.com>,
        Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
Subject: Re: checkpatch.pl should suggest __section

On Fri, 2019-08-09 at 15:21 -0700, Nick Desaulniers wrote:
> Hi Joe,
> While debugging:
> https://github.com/ClangBuiltLinux/linux/issues/619
> we found a bunch of places where __section is not used but could be,
> and uses a string literal when it probably should not be.
> 
> Just a thought that maybe checkpatch.pl could warn if
> `__attribute__((section` appeared in the added diff, and suggest
> __section? Then further warn to not use `""` for the section name?

Hmm, that makes me wonder about the existing __section uses
_with_ a quote are actually in the proper sections.

$ git grep -n -P '\b__section\s*\(\s*"'
arch/arm64/kernel/smp_spin_table.c:22:volatile unsigned long __section(".mmuoff.data.read")
arch/s390/boot/startup.c:49:static struct diag210 _diag210_tmp_dma __section(".dma.data");
include/linux/compiler.h:27:                            __section("_ftrace_annotated_branch")   \
include/linux/compiler.h:63:            __section("_ftrace_branch")             \
include/linux/compiler.h:121:#define __annotate_jump_table __section(".rodata..c_jump_table")
include/linux/compiler.h:158:   __section("___kentry" "+" #sym )                        \
include/linux/compiler.h:301:   static void * __section(".discard.addressable") __used \
include/linux/export.h:107:     static int __ksym_marker_##sym[0] __section(".discard.ksym") __used
include/linux/srcutree.h:127:           __section("___srcu_struct_ptrs") = &name

Maybe there should also be a __section("<foo>") test too.

Anyway, how about:
---
 scripts/checkpatch.pl | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 1cdacb4fd207..8e6693ca772c 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -5901,6 +5901,15 @@ sub process {
 			     "__aligned(size) is preferred over __attribute__((aligned(size)))\n" . $herecurr);
 		}
 
+# Check for __attribute__ section, prefer __section (without quotes)
+		if ($realfile !~ m@\binclude/uapi/@ &&
+		    $line =~ /\b__attribute__\s*\(\s*\(.*_*section_*\s*\(\s*("[^"]*")/) {
+			my $old = substr($rawline, $-[1], $+[1] - $-[1]);
+			my $new = substr($old, 1, -1);
+			WARN("PREFER_SECTION",
+			     "__section($new) is preferred over __attribute__((section($old)))\n" . $herecurr);
+		}
+
 # Check for __attribute__ format(printf, prefer __printf
 		if ($realfile !~ m@\binclude/uapi/@ &&
 		    $line =~ /\b__attribute__\s*\(\s*\(\s*format\s*\(\s*printf/) {