linux-kernel - Re: [PATCH RFC v2] checkpatch: extend attributes check to handle more patterns

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: <2a3b90ee-b9bd-2586-9d68-45cbf7e499a9@gmail.com>
Date:   Sat, 24 Oct 2020 00:44:49 +0530
From:   Dwaipayan Ray <dwaipayanray1@...il.com>
To:     Joe Perches <joe@...ches.com>
Cc:     linux-kernel-mentees@...ts.linuxfoundation.org,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Lukas Bulwahn <lukas.bulwahn@...il.com>
Subject: Re: [PATCH RFC v2] checkpatch: extend attributes check to handle more
 patterns


>>> And you could check using
>>>
>>>          $line =~ /__attribute__\s*\(\s*($balanced_parens)\s*)/
>>>
>>> and check for all attributes in $1 after
>>> stripping the leading and trailing parens
>>> and any leading and trailing underscores
>>> from each attribute.
>>>
>>> And you only need one hash and you should
>>> check for existence of the key rather than
>>> have multiple lists.
>>>
>>>          if (exists($attributes($attr))) {
>>>
>> Okay thanks!
>> But what should be done for the attributes which are
>> parameterized, like __aligned__(x). Should all the __
>> for these entries be trimmed too?
> yes
>
>> There are also
>> cases where there are multiple versions like:
>>
>> __aligned__
>> __aligned__(x)
> $ git grep __aligned__ | grep -v -P '__aligned__\s*\('
>
> AFAIK: There is only one use of bare __aligned__ and
> that's in compiler_attributes.h
>
>> To help differentiate between them what can be done?
>> Should i make the keys as:
>>
>> aligned
>> aligned__(
>>
>> instead of
>>
>> __aligned__
>> __aligned__(
> Just use aligned
>
> Just fyi:
>
> these are the uses of __attribute__ in the kernel
> with all the underscores and spaces removed so there's
> some value in finding the multiple actual attributes .


Hi,
I modified the check to check the attributes from the map.
There are two checks - one for the normal attributes and
one for the ones with arguments, which needs just a bit more processing.

So attributes like __packed__ as well as those like
__aligned__(x) are handled.

What do you think?

---
+            $line =~ /__attribute__\s*\(\s*($balanced_parens)\s*\)/) {
+            my $attr = trim($1);
+            $attr =~ s/\(\s*_*(.*)\)/$1/;
+            while($attr =~ s/(.*)_$/$1/) {}  # Remove trailing underscores
+
+            my %attr_list = (
+                "alias"            => "__alias",
+                "aligned"        => "__aligned",
+                "always_inline"     => "__always_inline",
+                "assume_aligned"    => "__assume_aligned",
+                "cold"            => "__cold",
+                "const"            => "__const",
+                "copy"            => "__copy",
+                "designated_init"        => "__designated_init",
+                "externally_visible"    => "__visible",
+                "fallthrough"        => "fallthrough",
+                "gnu_inline"        => "__gnu_inline",
+                "malloc"        => "__malloc",
+                "mode"            => "__mode",
+                "no_caller_saved_registers" => 
"__no_caller_saved_registers",
+                "noclone"        => "__noclone",
+                "noinline"        => "noinline",
+                "nonstring"        => "__nonstring",
+                "noreturn"        => "__noreturn",
+                "packed"        => "__packed",
+                "pure"            => "__pure",
+                "used"            => "__used"
+            );
+
+            if (exists($attr_list{$attr})) {
+                my $new = $attr_list{$attr};
+                WARN("PREFER_DEFINED_ATTRIBUTE_MACRO",
+                     "$new is preffered over 
__attribute__((__${attr}__))\n" . $herecurr);
+            }
+
+            # Check for attributes with parameters, like copy__(symbol)
+            if ($attr =~ /(.*)__(\(.*\))/) {
+                if (exists($attr_list{$1})) {
+                    my $new = $attr_list{$1};
+                    WARN("PREFER_DEFINED_ATTRIBUTE_MACRO",
+                         "$new$2 is preffered over 
__attribute__((__${attr}))\n" . $herecurr);
+                }
+            }
---

If this is okay I would like to send in a proper v3.

Thanks,
Dwaipayan.