linux-kernel - Re: [PATCH V3] wifi: rtlwifi: Drastically reduce the attempts to read efuse in case of failures

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: <5f87f72b-8768-50bc-4c0e-44c8fc12f5c4@igalia.com>
Date: Fri, 1 Nov 2024 16:29:41 -0300
From: "Guilherme G. Piccoli" <gpiccoli@...lia.com>
To: Ping-Ke Shih <pkshih@...ltek.com>
Cc: "kvalo@...nel.org" <kvalo@...nel.org>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
 "kernel@...ccoli.net" <kernel@...ccoli.net>,
 "kernel-dev@...lia.com" <kernel-dev@...lia.com>,
 "rtl8821cerfe2@...il.com" <rtl8821cerfe2@...il.com>,
 "stable@...r.kernel.org" <stable@...r.kernel.org>,
 "syzbot+edd9fe0d3a65b14588d5@...kaller.appspotmail.com"
 <syzbot+edd9fe0d3a65b14588d5@...kaller.appspotmail.com>,
 "linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>
Subject: Re: [PATCH V3] wifi: rtlwifi: Drastically reduce the attempts to read
 efuse in case of failures

On 31/10/2024 23:26, Ping-Ke Shih wrote:
>> [...]
> Have you run checkpatch before submission? Above two information can be in
> formal form as suggestions of checkpatch. 

Yes, I always do that! Happens that checkpatch sometimes provides good
advice (or even point to genuine errors), but sometimes...it's OK to
ignore the warnings, specially if we have a reason. Below I detail more
about my reasons:

> 
> WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?)
> #55:

It's like 76 long line, and helped readability on the commit message. In
any case, I'll refactor this one in V4, but notice that it'll continue
complaining because of point "[0]". That line has 80 or 81 chars, don't
recall, but if I reduce it by removing the word "Commit", for example,
checkpatch complains I'm writing a commit reference out of preferred
format heh

So, lose-lose scenario, I can't make the tool fully happy here xD

> WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report
> #72:

This advice would make sense..weren't it for the fact that this
Syzkaller issue is already closed heh

As I mentioned in the commit message, the main issue with this
reproducer is a race in the uevent path vs driver shutdown, addressed by
other commit, hence the Syzkaller report is fixed and closed.
But...this patch fixes a secondary issue there, and pointing to the
Syzkaller issue is useful first to give it credit, since both issues
were found by the tool, but also to point to the reproducer, so I kept
the Reported tag, but not the Closes one =)

> WARNING: The commit message has 'syzkaller', perhaps it also needs a 'Fixes:' tag?
> 

The fixes tag would point to the driver addition, ~10y ago. Stable bots
would attempt to backport it for all releases, which I think maybe is
not necessary. This is a small issue restricted to (old?) USB devices
and emulated devices (in reproducers), so in my commit message instead
of adding a Fixes tag, I've added Cc to stable with my suggestion of the
versions to backport (6.1.y and 6.6.y basically).

That decision is up to you and other maintainers, so feel free to chime
in if you prefer to backport to all releases or even *not* backport it
at all, I just provided a suggestion based on my understanding about the
issue and the relevance of this fix =]

Cheers,

Guilherme