netdev - Re: [PATCH, net] ibmvnic: fix firmware version when no firmware level has been provided by the VIOS server

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:   Fri, 2 Feb 2018 09:55:51 -0800
From:   Tyrel Datwyler <tyreld@...ux.vnet.ibm.com>
To:     Desnes Augusto Nunes do Rosário 
        <desnesn@...ux.vnet.ibm.com>, netdev@...r.kernel.org
Cc:     linuxppc-dev@...ts.ozlabs.org, jallen@...ux.vnet.ibm.com,
        tlfalcon@...ux.vnet.ibm.com, nfont@...ux.vnet.ibm.com
Subject: Re: [PATCH, net] ibmvnic: fix firmware version when no firmware level
 has been provided by the VIOS server

On 02/02/2018 06:37 AM, Desnes Augusto Nunes do Rosário wrote:
> Hello Tyrel,
> 
> I concur with your observations, but since this patch has already been merged, I'll address them in another patch.

Fair enough. I didn't realize David had already merged it till after I sent my review.

-Tyrel

> 
> Thank you for your review,
> 
> On 02/01/2018 07:02 PM, Tyrel Datwyler wrote:
>> On 02/01/2018 10:04 AM, Desnes Augusto Nunes do Rosario wrote:
>>> Older versions of VIOS servers do not send the firmware level in the VPD
>>> buffer for the ibmvnic driver. Thus, not only the current message is mis-
>>> leading but the firmware version in the ethtool will be NULL. Therefore,
>>> this patch fixes the firmware string and its warning.
>>>
>>> Fixes: 4e6759be28e4 ("ibmvnic: Feature implementation of VPD for the ibmvnic driver")
>>>
>>> Signed-off-by: Desnes A. Nunes do Rosario <desnesn@...ux.vnet.ibm.com>
>>> ---
>>>   drivers/net/ethernet/ibm/ibmvnic.c | 6 +++++-
>>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
>>> index b65f5f3ac034..2b3e71b63a7a 100644
>>> --- a/drivers/net/ethernet/ibm/ibmvnic.c
>>> +++ b/drivers/net/ethernet/ibm/ibmvnic.c
>>> @@ -3290,7 +3290,11 @@ static void handle_vpd_rsp(union ibmvnic_crq *crq,
>>>        */
>>>       substr = strnstr(adapter->vpd->buff, "RM", adapter->vpd->len);
>>>       if (!substr) {
>>> -        dev_info(dev, "No FW level provided by VPD\n");
>>> +        dev_info(dev, "Warning - No FW level has been provided in the VPD buffer by the VIOS Server\n");
>>> +        ptr = strncpy((char *)adapter->fw_version, "N/A",
>>
>> Is "N/A" the right thing to report? Would something like "Unknown" or "Unreported" be better?
>>
>>> +                  3 * sizeof(char));
>>> +        if (!ptr)
>>> +            dev_err(dev, "Failed to inform that firmware version is unavailable to the adapter\n");
>>
>> The sentence structure here seems awkward. I would probably just get rid of this error and this one later in the function.
>>
>>     dev_err(dev, "Failed to isolate FW level string\n");
>>
>> Instead just check and report if adapter->fw_version == NULL in the complete: label section.
>>
>> -Tyrel
>>
>>>           goto complete;
>>>       }
>>>
>>
>