linux-kernel - Re: [linux-safety] [PATCH] usb: host: ehci-sched: add comment about find

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: <alpine.DEB.2.21.2010122029540.17866@felia>
Date:   Mon, 12 Oct 2020 20:49:39 +0200 (CEST)
From:   Lukas Bulwahn <lukas.bulwahn@...il.com>
To:     Sudip Mukherjee <sudipm.mukherjee@...il.com>
cc:     Lukas Bulwahn <lukas.bulwahn@...il.com>,
        Alan Stern <stern@...land.harvard.edu>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        linux-safety@...ts.elisa.tech, linux-usb@...r.kernel.org
Subject: Re: [linux-safety] [PATCH] usb: host: ehci-sched: add comment about
 find_tt() not returning error

On Mon, 12 Oct 2020, Sudip Mukherjee wrote:

> 
> >
> > I am wondering if such comment deserves to be included if we cannot check
> > its validity later...
> 
> I am failing to understand why will you not be able to check its
> validity later. You just need to read the code to check it.
>

Well, I meant automatically checking the validity with some tool, like a 
tool (code analyzer, model checker, ...) that can check if a certain 
annotation holds.

> >
> > I would prefer a simple tool that could check that your basic assumption
> > on the code is valid and if it the basic assumption is still valid,
> > just shut up any stupid tool that simply does not get that these calls
> > here cannot return any error.
> >
> > We encountered this issue because of clang analyzer complaining, but it is
> > clear that it is a false positive of that (smart but) incomplete tool.
> 
> I dont think it is a false positive. The error return value is not
> checked and that is true. Only that it is not applicable because of
> the way the coding is done.
>

Maybe we have different understandings of a false positive reported by a 
tool...

My definitions are in the LPC 2020 presentation, Maintaining results from 
static analysis collaboratively, slide 4:

https://linuxplumbersconf.org/event/7/contributions/700/attachments/606/1088/LPC2020-Static-Analysis.pdf

So, for me, what you wrote above is exactly the definition of a
"False Tool Finding (False Positive, Type A)".

Given that Alan will accept to add a comment in the code, it is a "True 
Tool-Induced Change (True Positive, Type B)" because we can actually 
provide a reasonable patch based on what the tool reported (even though it 
is just supportive documentation, no change in the code).

> >
> > Do you intend to add comment for all false positives from all tools or are
> > we going to find a better solution than that?
> 
> I think tools will always give you some false positives and you will
> need to read the code to know if its false positive or not. I dont
> think there is any tool yet which will only give true positives.
>

Well, given humans provide some annotations to initially false positives, 
there is a fair chance that a tool only gives true positives after the 
annotations.

With 'just comments', the tool will continue to complain and we will need 
to read the code once again every time we did not know that that case was 
a false positive. That going to happen often, namely every time, a new 
developer looks at the tool report.

Let us check if an annotation can help beyond the current comment. This 
annotation can of course be provided later independently with another 
patch.

Lukas