linux-kernel - Re: [linux-safety] [PATCH] kernel: trace: Remove deadstore in trace

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] [day] [month] [year] [list]

Message-ID: <ee9d9a8c-53de-01d2-7489-6f650857013b@codethink.co.uk>
Date:   Thu, 10 Dec 2020 11:29:13 +0000
From:   Milan Lakhani <milan.lakhani@...ethink.co.uk>
To:     Lukas Bulwahn <lukas.bulwahn@...il.com>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-safety@...ts.elisa.tech
Subject: Re: [linux-safety] [PATCH] kernel: trace: Remove deadstore in
 trace_uprobe.c


On 10/12/2020 05:47, Lukas Bulwahn wrote:
> On Wed, Dec 9, 2020 at 2:17 PM Milan Lakhani
> <milan.lakhani@...ethink.co.uk> wrote:
>> In trace_uprobe.c, trace_uprobe_create assigns ret to 0 but then never
>> uses this value.
>>
> Milan, the patch makes sense, but I fear you did not run
> ./scripts/get_maintainers.pl because you did not include any specific
> maintainer as recipient.

So I did actually run get_maintainers.pl but somehow in my text editor 
where I had pasted the e-mail addresses they had got onto two separate 
lines, and git send-email must have only taken the first line. I'll have 
to get the e-mails right first time next time, to move onto checking 
documentation!

> The patch subject could be: remove unneeded initialization (instead of
> the very generic "dead store" term).
Updated this, thanks
>
> It is also interesting to see who added this initialization; was it
> unneeded since the existence of this function, did it become obsolete
> at some point in time due to refactoring?

The line was unneeded since the function was created in 2012 by Srikar 
Dronamraju in f3f096cfe, there have actually somehow been six commits to 
the lines between the initial assignment and reassignment of ret but 
they have all left the unnecessary initialization there!

>
> Run ./scripts/get_maintainers.pl  and please CC: me; then you will get
> your Reviewed-by: tag.
>
> Lukas
>
>> Signed-off-by: Milan Lakhani <milan.lakhani@...ethink.co.uk>
>> ---
>>   kernel/trace/trace_uprobe.c | 1 -
>>   1 file changed, 1 deletion(-)
>>
>> diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
>> index 3cf7128..c7c7070 100644
>> --- a/kernel/trace/trace_uprobe.c
>> +++ b/kernel/trace/trace_uprobe.c
>> @@ -541,7 +541,6 @@ static int trace_uprobe_create(int argc, const char **argv)
>>          bool is_return = false;
>>          int i, ret;
>>
>> -       ret = 0;
>>          ref_ctr_offset = 0;
>>
>>          switch (argv[0][0]) {
>> --
>> 2.7.4
>>