netdev - Re: [PATCH] ipw2x00: Remove a memory allocation failure log message

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, 24 Apr 2020 07:33:33 +0300
From:   Kalle Valo <kvalo@...eaurora.org>
To:     Christophe JAILLET <christophe.jaillet@...adoo.fr>
Cc:     Sergei Shtylyov <sergei.shtylyov@...entembedded.com>,
        stas.yakovlev@...il.com, davem@...emloft.net,
        linux-wireless@...r.kernel.org, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, kernel-janitors@...r.kernel.org
Subject: Re: [PATCH] ipw2x00: Remove a memory allocation failure log message

Christophe JAILLET <christophe.jaillet@...adoo.fr> writes:

> Le 23/04/2020 à 11:46, Sergei Shtylyov a écrit :
>> Hello!
>>
>> On 23.04.2020 10:58, Christophe JAILLET wrote:
>>
>>> Axe a memory allocation failure log message. This message is useless and
>>> incorrect (vmalloc is not used here for the memory allocation)
>>>
>>> This has been like that since the very beginning of this driver in
>>> commit 43f66a6ce8da ("Add ipw2200 wireless driver.")
>>>
>>> Signed-off-by: Christophe JAILLET <christophe.jaillet@...adoo.fr>
>>> ---
>>>   drivers/net/wireless/intel/ipw2x00/ipw2200.c | 5 ++---
>>>   1 file changed, 2 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/intel/ipw2x00/ipw2200.c
>>> b/drivers/net/wireless/intel/ipw2x00/ipw2200.c
>>> index 60b5e08dd6df..30c4f041f565 100644
>>> --- a/drivers/net/wireless/intel/ipw2x00/ipw2200.c
>>> +++ b/drivers/net/wireless/intel/ipw2x00/ipw2200.c
>>> @@ -3770,10 +3770,9 @@ static int ipw_queue_tx_init(struct ipw_priv
>>> *priv,
>>>       struct pci_dev *dev = priv->pci_dev;
>>>         q->txb = kmalloc_array(count, sizeof(q->txb[0]), GFP_KERNEL);
>>> -    if (!q->txb) {
>>> -        IPW_ERROR("vmalloc for auxiliary BD structures failed\n");
>>> +    if (!q->txb)
>>>           return -ENOMEM;
>>> -    }
>>> +
>>
>>    No need for this extra empty line.
>
>
> That's right, sorry about that.
>
> Can it be fixed when/if the patch is applied, or should I send a V2?

Please send v2.

> If a V2 is required, should kcalloc be used, as pointed out by Joe Perches?
> (personally, If the code works fine as-is, I don't think it is
> required, but it can't hurt)

There's always the risk of regressions, which happens even with cleanup
patches so hurting is always possible :)

I can take a patch changing the allocation but please do it in a
separate patch. Though personally I wouldn't bother, ipw2x00 is an old
driver and not being actively developed anymore.

-- 
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches