netdev - Re: [PATCH v3 net-next 2/4] ionic: extract common bits from ionic

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8baedaad-4b4d-922f-fd12-14d68375bb13@amd.com>
Date: Wed, 19 Jul 2023 17:04:16 -0700
From: Shannon Nelson <shannon.nelson@....com>
To: Simon Horman <simon.horman@...igine.com>
Cc: netdev@...r.kernel.org, davem@...emloft.net, kuba@...nel.org,
 idosch@...sch.org, brett.creeley@....com, drivers@...sando.io
Subject: Re: [PATCH v3 net-next 2/4] ionic: extract common bits from
 ionic_probe

On 7/19/23 10:48 AM, Simon Horman wrote:
> 
> On Mon, Jul 17, 2023 at 09:59:59AM -0700, Shannon Nelson wrote:
>> Pull out some chunks of code from ionic_probe() that will
>> be common in rebuild paths.
>>
>> Signed-off-by: Shannon Nelson <shannon.nelson@....com>
> 
> ...
> 
>> +static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>> +{
>> +     struct device *dev = &pdev->dev;
>> +     struct ionic *ionic;
>> +     int num_vfs;
>> +     int err;
>> +
>> +     ionic = ionic_devlink_alloc(dev);
>> +     if (!ionic)
>> +             return -ENOMEM;
>> +
>> +     ionic->pdev = pdev;
>> +     ionic->dev = dev;
>> +     pci_set_drvdata(pdev, ionic);
>> +     mutex_init(&ionic->dev_cmd_lock);
>> +
>> +     /* Query system for DMA addressing limitation for the device. */
>> +     err = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(IONIC_ADDR_LEN));
>> +     if (err) {
>> +             dev_err(dev, "Unable to obtain 64-bit DMA for consistent allocations, aborting.  err=%d\n",
>> +                     err);
>> +             goto err_out;
>> +     }
>> +
>> +     err = ionic_setup_one(ionic);
>> +     if (err)
>> +             goto err_out;
>> +
>>        /* Allocate and init the LIF */
>>        err = ionic_lif_size(ionic);
>>        if (err) {
>>                dev_err(dev, "Cannot size LIF: %d, aborting\n", err);
>> -             goto err_out_port_reset;
>> +             goto err_out_pci;
> 
> Hi Shannon,
> 
> Prior to this patch, if this error occurred then the following cleanup
> would occur.
> 
>          ionic_port_reset(ionic);
>          ionic_reset(ionic);
>          ionic_dev_teardown(ionic);
>          ionic_clear_pci(ionic);
>          ionic_debugfs_del_dev(ionic);
>          mutex_destroy(&ionic->dev_cmd_lock);
>          ionic_devlink_free(ionic);
> 
> With this patch I am assuming that the same setup has occurred at
> this point (maybe I am mistaken). But with the following cleanup on error.
> 
>          ionic_clear_pci(ionic);
>          mutex_destroy(&ionic->dev_cmd_lock);
>          ionic_devlink_free(ionic);
> 
> I feel that I'm reading this wrong.

You read that right.  The port_reset and reset are superflous so are 
dropped here.  However, that dev_teardown() does need to happen to be 
sure we don't leak a couple CMB related things.  I'll add that.

Thanks,
sln

> 
>>        }
>>
>>        err = ionic_lif_alloc(ionic);
>> @@ -354,17 +375,9 @@ static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>>        ionic->lif = NULL;
>>   err_out_free_irqs:
>>        ionic_bus_free_irq_vectors(ionic);
>> -err_out_port_reset:
>> -     ionic_port_reset(ionic);
>> -err_out_reset:
>> -     ionic_reset(ionic);
>> -err_out_teardown:
>> -     ionic_dev_teardown(ionic);
>> -err_out_clear_pci:
>> +err_out_pci:
>>        ionic_clear_pci(ionic);
>> -err_out_debugfs_del_dev:
>> -     ionic_debugfs_del_dev(ionic);
>> -err_out_clear_drvdata:
>> +err_out:
>>        mutex_destroy(&ionic->dev_cmd_lock);
>>        ionic_devlink_free(ionic);
>>
>> --
>> 2.17.1
>>
>>