linux-kernel - Re: [PATCH v7 2/4] firmware: raspberrypi: Introduce vl805 init routine

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]

Date:   Mon, 4 May 2020 17:52:34 -0500
From:   Bjorn Helgaas <helgaas@...nel.org>
To:     Nicolas Saenz Julienne <nsaenzjulienne@...e.de>
Cc:     Stefan Wahren <wahrenst@....net>, linux-usb@...r.kernel.org,
        linux-rpi-kernel@...ts.infradead.org,
        linux-arm-kernel@...ts.infradead.org, tim.gover@...pberrypi.org,
        linux-pci@...r.kernel.org, f.fainelli@...il.com,
        Greg KH <gregkh@...uxfoundation.org>,
        linux-kernel@...r.kernel.org, RayJui <rjui@...adcom.com>,
        ScottBranden <sbranden@...adcom.com>,
        bcm-kernel-feedback-list@...adcom.com
Subject: Re: [PATCH v7 2/4] firmware: raspberrypi: Introduce vl805 init
 routine

On Mon, May 04, 2020 at 10:59:29AM +0200, Nicolas Saenz Julienne wrote:
> On Sat, 2020-05-02 at 11:05 +0200, Stefan Wahren wrote:
> > > +	pci_read_config_dword(pdev, VL805_PCI_CONFIG_VERSION_OFFSET, &version);
> > pci_read_config_dword() can fail, we might want to store the return value?
> 
> I planned on doing that, but realised that the vast majority of
> pci_read_config_*() users pass on checking for errors.
> 
> Bjorn, any rule of thumb on when to check for errors here?

Not really.  It *can* fail, for sure.  If it does fail, you normally
get ~0 data, which means you would skip the firmware load, do another
config read (which probably also returns ~0) and print firmware
version ffffffff, and the device probably won't work.

But checking doesn't get you much other than a better error message.

Personally I probably wouldn't bother because it clutters the code so
much for so little benefit.

Bjorn