linux-hardening - Re: [PATCH -next v3 01/10] coccinelle: Add rules to find str_true

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: <21356a02-c49d-4afe-a14c-f96fdb5342ed@huawei.com>
Date: Fri, 27 Sep 2024 17:56:09 +0800
From: Hongbo Li <lihongbo22@...wei.com>
To: Julia Lawall <Julia.Lawall@...ia.fr>
CC: <kees@...nel.org>, <andy@...nel.org>, <nicolas.palix@...g.fr>,
	<linux-hardening@...r.kernel.org>, <cocci@...ia.fr>
Subject: Re: [PATCH -next v3 01/10] coccinelle: Add rules to find
 str_true_false() replacements



On 2024/9/23 18:24, Julia Lawall wrote:
> Thanks for testing it. I will see if there is some other way to improve the performance.

May be every rules in the same file are executed sequentially cost a lot?

Thanks,
Hongbo

> Sent from my iPhone
> 
>> On 23 Sep 2024, at 09:01, Hongbo Li <lihongbo22@...wei.com> wrote:
>>
>> 
>>
>>> On 2024/9/19 14:25, Julia Lawall wrote:
>>>> On Wed, 11 Sep 2024, Hongbo Li wrote:
>>>> After str_true_false() has been introduced in the tree,
>>>> we can add rules for finding places where str_true_false()
>>>> can be used. A simple test can find over 10 locations.
>>>>
>>>> Signed-off-by: Hongbo Li <lihongbo22@...wei.com>
>>>> ---
>>>>   scripts/coccinelle/api/string_choices.cocci | 19 +++++++++++++++++++
>>>>   1 file changed, 19 insertions(+)
>>>>
>>>> diff --git a/scripts/coccinelle/api/string_choices.cocci b/scripts/coccinelle/api/string_choices.cocci
>>>> index 5e729f187f22..6942ad7c4224 100644
>>>> --- a/scripts/coccinelle/api/string_choices.cocci
>>>> +++ b/scripts/coccinelle/api/string_choices.cocci
>>>> @@ -85,3 +85,22 @@ e << str_down_up_r.E;
>>>>   @@
>>>>
>>>>   coccilib.report.print_report(p[0], "opportunity for str_down_up(%s)" % e)
>>>> +
>>>> +@..._true_false depends on patch@
>>>> +expression E;
>>>> +@@
>>>> +-      ((E) ? "true" : "false")
>>>> ++      str_true_false(E)
>>>> +
>>>> +@..._true_false_r depends on !patch exists@
>>>> +expression E;
>>>> +position P;
>>>> +@@
>>>> +*      ((E@P) ? "true" : "false")
>>> Hello,
>>> The semantic patch is quite slow.  Actually it tests a large number of
>>> cases, eg where the parentheses are present and where they are not.
>>> A small optimization is possible in the non-patch case.  The outer
>>> parentheses are not needed, because you will already get the same
>>> information whether they are there or not.
>>> In contrast, for the patch case, the outer parentheses are needed, because
>>> if they are there we want to remove them, since they are not needed for
>>> the function call.
>>> Could you update the depends on !patch cases to remove the outer
>>> parentheses?
>>
>> You mean in non-patch case, we could just write like the following?:
>>
>> +@..._true_false_r depends on !patch exists@
>> +expression E;
>> +position P;
>> +@@
>> +*      (E@P) ? "true" : "false"
>>
>> I have tested in my machine. The impact of this parenthesis on performance is very minimal.
>>
>> With parentheses, the time on driver/ costs:
>>
>> real    1m41.696s
>> user    85m24.069s
>> sys    1m8.891s
>>
>> Without parentheses, the time on driver/ costs:
>>
>> real    1m40.438s
>> user    85m53.987s
>> sys    1m7.981s
>>
>>
>> Thanks,
>> Hongbo
>>
>>> Also, just one patch would be fine.  There are many changes, but they are
>>> all sort of the same, so it would be easier just to see them all at once.
>>> thanks,
>>> julia
>>>> +
>>>> +@...ipt:python depends on report@
>>>> +p << str_true_false_r.P;
>>>> +e << str_true_false_r.E;
>>>> +@@
>>>> +
>>>> +coccilib.report.print_report(p[0], "opportunity for str_true_false(%s)" % e)
>>>> --
>>>> 2.34.1
>>>>
>>>>
>