linux-kernel - Re: [PATCH] usb: host: fhci-hcd: annotate PIPE

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: <f0e804c2-f7a3-da91-9929-38ac7f017081@rasmusvillemoes.dk>
Date:   Mon, 17 Feb 2020 20:44:04 +0100
From:   Rasmus Villemoes <linux@...musvillemoes.dk>
To:     Joe Perches <joe@...ches.com>,
        "Gustavo A. R. Silva" <gustavo@...eddedor.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     Timur Tabi <timur@...nel.org>, Li Yang <leoyang.li@....com>,
        Anton Vorontsov <avorontsov@...mvista.com>,
        kbuild test robot <lkp@...el.com>, linux-usb@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] usb: host: fhci-hcd: annotate PIPE_CONTROL switch case
 with fallthrough

On 17/02/2020 18.33, Joe Perches wrote:
> On Mon, 2020-02-17 at 11:12 -0600, Gustavo A. R. Silva wrote:
>>
>>>> Reported-by: kbuild test robot <lkp@...el.com>
>>>> Fixes: 5a35435ef4e6 (soc: fsl: qe: remove PPC32 dependency from CONFIG_QUICC_ENGINE)
>>>> Fixes: a035d552a93b (Makefile: Globally enable fall-through warning)
>>
>> By the way, the "Fixes" tag above makes no sense. There is nothing wrong about
>> that commit. It just enabled the fall-through warning globally. Why would you
>> "fix" that?"

Depends on whether you consider a change that introduces a warning in an
otherwise warning-free build a regression or not. That commit claimed

    Now that all the fall-through warnings have been addressed in the
    kernel, enable the fall-through warning globally.

but as I explained below the fold, any CONFIG_PPC32+CONFIG_USB_FHCI_HCD
.config grew a warning due to a035d552a93b. So at least in that sense
there is something wrong about that commit - the above claim is simply
false. Please note that I don't expect anybody to ever be able to
actually cover everything before doing something like what a035d552a93b
does, so I'm not complaining, just explaining.

Then I introduced a change which made that code compile for a ppc64
allmodconfig, which apparently 0day does cover, which is why I added
that other tag.

> There could be some effort made to better specify when "Fixes:"
> tags should be used.

Indeed. I explicitly chose not to cc stable because I don't think it's
for -stable. But in case somebody (or Sasha's ML) decides it is, I went
out of my way to include relevant commits and an explanation for the
somewhat odd dual Fixes:. So no, I don't think Fixes implies or should
imply Cc stable - and I think this is all consistent with
submitting-patches.rst:

  Patches that fix a severe bug in a released kernel should be directed
toward the stable maintainers...

and

  A Fixes: tag indicates that the patch fixes an issue in a previous commit.

Nothing says that Fixes is reserved for -stable material.

> I believe "Fixes:" should be used only when changes have some
> runtime impact. 

Perhaps. But it's hard to make the rules completely rigid - suppose
commit A does fix a real bug and is backported, however, in some configs
it introduces some warnings; that gets fixed by B which doesn't change
generated code. Should B be backported, or should the -stable tree(s)
live with those warnings?

"Fixes:" should not be used for changes that
> just silence compiler warnings using W=<123>.

I tend to agree, but that's completely irrelevant in this case, as this
is not a warning that only appears for W=<123>.

Rasmus