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]
Date:   Wed, 19 Jan 2022 08:15:10 +0100
From:   Lukas Bulwahn <>
To:     Yuntao Wang <>
Cc:     Steven Rostedt <>,
        Ingo Molnar <>,
        Linux Kernel Mailing List <>
Subject: Re: [PATCH] tracing: Remove redundant assignment to variable ret

On Wed, Jan 19, 2022 at 5:18 AM Yuntao Wang <> wrote:
> On Mon, Jan 17, 2022 at 3:47 PM Lukas Bulwahn <> wrote:
> > Dear Yuntao,
> >
> >
> > when you consider removing dead-store assignments guided by some static
> > analyzer, you need to check if the code you are looking at is actually
> > missing an error-handling branch.
> >
> > In this case, ftrace_process_locs() may return -ENOMEM, and the caller
> > needs to appropriately deal with this error return code. Your patch
> > does not change the code at all, i.e., the compiled object code is the
> > same as after the patch as before.
> >
> > Think about how to deal appropriately with the -ENOMEM return in this
> > caller and submit a patch that implements the right error-handling
> > branch or argue in your commit message why that is not needed at all.
> >
> > If you do not understand or cannot check such basic code properties for
> > dead-store assignments, it might be better to work on some other aspect
> > and area of the kernel repository. E.g., the kernel documentation build
> > also has a few warnings that deserve patches to be fixed.
> >
> >
> > Best regards,
> >
> > Lukas
> Dear Lukas,
> Thanks for your reply.
> Actually, I had read the source code carefully and noticed the possible
> error return code -ENOMEM of the ftrace_process_locs() function.
> At first I was going to implement an error-handling branch as you said,
> but after digging into more details, I discovered:
> - The ftrace_init() function did not handle the error return code of the ftrace_process_locs() function since the first version.
> - The ftrace_module_init() function did not handle it either.

This is certainly important information on the rationale, and hence,
this needs to go into the commit message explaining why you propose
this change.

Now, you should also explain: why do you consider it not a problem
that this error situation -ENOMEM is not handled by the caller?

And if so, why should ftrace_process_locs() even return an error code
if this error return is not considered?

Your commit message should really explain this reasoning.

> To keep consistent with the existing code, I just removed the assignment
> in that patch.
> Maybe we should deal with the error return code more appropriately,
> at least print some warnings?

This might be one way of dealing with it.


Powered by blists - more mailing lists