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
| ||
|
Date: Tue, 25 Oct 2022 14:48:14 +0300 From: Leon Romanovsky <leon@...nel.org> To: Yang Yingliang <yangyingliang@...wei.com> Cc: netdev@...r.kernel.org, davem@...emloft.net, kuba@...nel.org Subject: Re: [PATCH net] net: fealnx: fix missing pci_disable_device() On Tue, Oct 25, 2022 at 07:25:48PM +0800, Yang Yingliang wrote: > > On 2022/10/25 18:27, Leon Romanovsky wrote: > > On Tue, Oct 25, 2022 at 05:01:58PM +0800, Yang Yingliang wrote: > > > On 2022/10/25 15:47, Leon Romanovsky wrote: > > > > On Mon, Oct 24, 2022 at 09:57:28PM +0800, Yang Yingliang wrote: > > > > > pci_disable_device() need be called while module exiting, switch > > > > > to use pcim_enable(), pci_disable_device() and pci_release_regions() > > > > > will be called in pcim_release() while unbinding device. > > > > I didn't understand the description at all, maybe people in CC will. > > > > Most likely, you wanted something like this: > > > After using pcim_enable_device(), pcim_release() will be called while > > > unbinding > > > device, pcim_release() calls both pci_release_regions() and > > > pci_disable_device(). > > I'm not sure that you can mix managed with unmanaged PCI APIs. > > > > Thanks > After pcim_enable_device() being called, the region_mask is set in > __pci_request_region(). pcim_release() will call pci_release_region() > because region_mask is set. > > Without device managed, the pci_release_region() and pci_disable_device() > is called at end of remove() function which is called in device_remove(), > and > with managed, they are called in device_unbind_cleanup() which is called > after device_remove(). So I think it's OK to use this management. Even so, why do you think that call in device_unbind_cleanup() is the right one for such an old driver? Thanks > > Thanks, > Yang > > > > > Thanks, > > > Yang > > > > diff --git a/drivers/net/ethernet/fealnx.c b/drivers/net/ethernet/fealnx.c > > > > index ed18450fd2cc..78107dd4aa57 100644 > > > > --- a/drivers/net/ethernet/fealnx.c > > > > +++ b/drivers/net/ethernet/fealnx.c > > > > @@ -690,6 +690,7 @@ static void fealnx_remove_one(struct pci_dev *pdev) > > > > pci_iounmap(pdev, np->mem); > > > > free_netdev(dev); > > > > pci_release_regions(pdev); > > > > + pci_disable_device(pdev); > > > > } else > > > > printk(KERN_ERR "fealnx: remove for unknown device\n"); > > > > > > > > > > > > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > > > > > Signed-off-by: Yang Yingliang <yangyingliang@...wei.com> > > > > > --- > > > > > drivers/net/ethernet/fealnx.c | 4 +--- > > > > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > > > > > > > diff --git a/drivers/net/ethernet/fealnx.c b/drivers/net/ethernet/fealnx.c > > > > > index ed18450fd2cc..fb139f295b67 100644 > > > > > --- a/drivers/net/ethernet/fealnx.c > > > > > +++ b/drivers/net/ethernet/fealnx.c > > > > > @@ -494,7 +494,7 @@ static int fealnx_init_one(struct pci_dev *pdev, > > > > > option = card_idx < MAX_UNITS ? options[card_idx] : 0; > > > > > - i = pci_enable_device(pdev); > > > > > + i = pcim_enable_device(pdev); > > > > > if (i) return i; > > > > > pci_set_master(pdev); > > > > > @@ -670,7 +670,6 @@ static int fealnx_init_one(struct pci_dev *pdev, > > > > > err_out_unmap: > > > > > pci_iounmap(pdev, ioaddr); > > > > > err_out_res: > > > > > - pci_release_regions(pdev); > > > > > return err; > > > > > } > > > > > @@ -689,7 +688,6 @@ static void fealnx_remove_one(struct pci_dev *pdev) > > > > > unregister_netdev(dev); > > > > > pci_iounmap(pdev, np->mem); > > > > > free_netdev(dev); > > > > > - pci_release_regions(pdev); > > > > > } else > > > > > printk(KERN_ERR "fealnx: remove for unknown device\n"); > > > > > } > > > > > -- > > > > > 2.25.1 > > > > > > > > > . > > .
Powered by blists - more mailing lists