linux-kernel - Re: [PATCH v1 01/12] gna: add driver module

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: <85blcgw5v6.fsf@linux.intel.com>
Date:   Fri, 19 Feb 2021 14:21:17 +0100
From:   Maciej Kwapulinski <maciej.kwapulinski@...ux.intel.com>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Arnd Bergmann <arnd@...db.de>,
        Jonathan Corbet <corbet@....net>,
        Derek Kiernan <derek.kiernan@...inx.com>,
        Dragan Cvetic <dragan.cvetic@...inx.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux Documentation List <linux-doc@...r.kernel.org>,
        Tomasz Jankowski <tomasz1.jankowski@...el.com>,
        Savo Novakovic <savox.novakovic@...el.com>,
        Jianxun Zhang <jianxun.zhang@...ux.intel.com>
Subject: Re: [PATCH v1 01/12] gna: add driver module


Andy Shevchenko <andy.shevchenko@...il.com> writes:

> On Tue, Feb 16, 2021 at 6:11 PM Maciej Kwapulinski
> <maciej.kwapulinski@...ux.intel.com> wrote:
>>
....
>> +err_clear_master:
>> +       pci_clear_master(pcidev);
>> +err_release_regions:
>> +       pci_release_regions(pcidev);
>> +end:
>> +       dev_err(&pcidev->dev, "gna probe failed with %d\n", ret);
>> +       return ret;
>
> These are all completely redundant.
>

following is refactor of gna_probe(), but without pci_release_regions(),
smatch (v7fcfe259) produces warning:
  drivers/misc/gna/gna_device.c:78 gna_probe() warn: 'pcidev' not
  released on lines: 56,65.

here's the code refactored:

int gna_probe(struct pci_dev *pcidev, const struct pci_device_id *pci_id)
{
	struct gna_private *gna_priv;
	int ret;

	ret = pcim_enable_device(pcidev);
	if (ret) {
		dev_err(&pcidev->dev, "pci device can't be enabled\n");
		return ret;
	}

	ret = pci_request_regions(pcidev, GNA_DRV_NAME);
	if (ret)
		return ret;

	ret = pci_set_dma_mask(pcidev, DMA_BIT_MASK(64));
	if (ret) {
		dev_err(&pcidev->dev, "pci_set_dma_mask returned error %d\n", ret);
		return ret;
	}

	pci_set_master(pcidev);

	/* init gna device */
	gna_priv = devm_kzalloc(&pcidev->dev, sizeof(*gna_priv), GFP_KERNEL);
	if (!gna_priv) {
		//pci_release_regions(pcidev);
		return -ENOMEM;                 // line 56
	}
	/* Map BAR0 */
	gna_priv->bar0.iostart = pci_resource_start(pcidev, 0);
	gna_priv->bar0.iosize = pci_resource_len(pcidev, 0);
	gna_priv->bar0.mem_addr = pcim_iomap(pcidev, 0, 0);
	if (!gna_priv->bar0.mem_addr) {
		//pci_release_regions(pcidev);
		dev_err(&pcidev->dev, "could not map BAR 0\n");
		return -EINVAL;               // line 65
	}

	dev_dbg(&pcidev->dev, "bar0 io start: 0x%llx\n", (unsigned long long)gna_priv->bar0.iostart);
	dev_dbg(&pcidev->dev, "bar0 io size: %llu\n", (unsigned long long)gna_priv->bar0.iosize);
	dev_dbg(&pcidev->dev, "bar0 memory address: %p\n", gna_priv->bar0.mem_addr);

	ret = gna_dev_init(gna_priv, pcidev, pci_id);
	if (ret) {
		dev_err(&pcidev->dev, "could not initialize gna private structure\n");
		return ret;
	}

	return 0;
}

I've also added 'noinline' directive to pci_release_regions(), to see if
it is called by the core code on "rmmod gna", but can't see the call.

Is the smatch tool that causes problems here?
Do You suggest other way to handle the problem?