netdev - Re: [PATCH net] net: liquidio: release resources when liquidio driver open failed

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:   Thu, 10 Nov 2022 18:04:36 +0800
From:   shaozhengchao <shaozhengchao@...wei.com>
To:     Leon Romanovsky <leon@...nel.org>
CC:     <netdev@...r.kernel.org>, <dchickles@...vell.com>,
        <sburla@...vell.com>, <fmanlunas@...vell.com>,
        <davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>,
        <pabeni@...hat.com>, <richardcochran@...il.com>,
        <rvatsavayi@...iumnetworks.com>, <gregkh@...uxfoundation.org>,
        <tseewald@...il.com>, <weiyongjun1@...wei.com>,
        <yuehaibing@...wei.com>
Subject: Re: [PATCH net] net: liquidio: release resources when liquidio driver
 open failed



On 2022/11/10 17:45, Leon Romanovsky wrote:
> On Thu, Nov 10, 2022 at 09:31:16AM +0800, Zhengchao Shao wrote:
>> When liquidio driver open failed, it doesn't release resources. Compile
>> tested only.
>>
>> Fixes: 5b07aee11227 ("liquidio: MSIX support for CN23XX")
>> Fixes: dbc97bfd3918 ("net: liquidio: Add missing null pointer checks")
>> Signed-off-by: Zhengchao Shao <shaozhengchao@...wei.com>
>> ---
>>   .../net/ethernet/cavium/liquidio/lio_main.c   | 40 ++++++++++++++++---
>>   1 file changed, 34 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/cavium/liquidio/lio_main.c b/drivers/net/ethernet/cavium/liquidio/lio_main.c
>> index d312bd594935..713689cf212c 100644
>> --- a/drivers/net/ethernet/cavium/liquidio/lio_main.c
>> +++ b/drivers/net/ethernet/cavium/liquidio/lio_main.c
>> @@ -1795,12 +1795,15 @@ static int liquidio_open(struct net_device *netdev)
>>   	ifstate_set(lio, LIO_IFSTATE_RUNNING);
>>   
>>   	if (OCTEON_CN23XX_PF(oct)) {
>> -		if (!oct->msix_on)
>> -			if (setup_tx_poll_fn(netdev))
>> -				return -1;
>> +		if (!oct->msix_on) {
>> +			ret = setup_tx_poll_fn(netdev);
>> +			if (ret)
>> +				goto err_poll;
>> +		}
>>   	} else {
>> -		if (setup_tx_poll_fn(netdev))
>> -			return -1;
>> +		ret = setup_tx_poll_fn(netdev);
>> +		if (ret)
>> +			goto err_poll;
>>   	}
> 
> Instead of this hairy code, you can squeeze everything into one if:
> 
> if (!OCTEON_CN23XX_PF(oct) || (OCTEON_CN23XX_PF(oct) && oct->msix_on)) {
>    ret = setup_tx_poll_fn(netdev);
>    if (ret)
>       ,,,
> 
> Thanks
Hi Leon：
	Thank you for your suggestion. I will modify it in V2.

Zhengchao Shao