| Message ID | 01446d2ccb91a578239915812f2b7dfbeb2882af.1703428183.git.christophe.jaillet@wanadoo.fr |
|---|---|
| State | New |
| Headers | show |
| Series | PCI: switchtec: Fix an error handling path in switchtec_pci_probe() | expand |
[+to Daniel, can you take a look? If you like this, I'd like to squash it into df25461119d9 and credit Christophe since that's not upstream yet] On Sun, Dec 24, 2023 at 03:30:01PM +0100, Christophe JAILLET wrote: > The commit in Fixes changed the logic on how resources are released and > introduced a new switchtec_exit_pci() that need to be called explicitly in > order to undo a corresponding switchtec_init_pci(). > > This was done in the remove function, but not in the probe. > > Fix the probe now. > > Fixes: df25461119d9 ("PCI: switchtec: Fix stdev_release() crash after surprise hot remove") > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > --- > drivers/pci/switch/switchtec.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c > index 1804794d0e68..5a4adf6c04cf 100644 > --- a/drivers/pci/switch/switchtec.c > +++ b/drivers/pci/switch/switchtec.c > @@ -1672,7 +1672,7 @@ static int switchtec_pci_probe(struct pci_dev *pdev, > rc = switchtec_init_isr(stdev); > if (rc) { > dev_err(&stdev->dev, "failed to init isr.\n"); > - goto err_put; > + goto err_exit_pci; > } > > iowrite32(SWITCHTEC_EVENT_CLEAR | > @@ -1693,6 +1693,8 @@ static int switchtec_pci_probe(struct pci_dev *pdev, > > err_devadd: > stdev_kill(stdev); > +err_exit_pci: > + switchtec_exit_pci(stdev); > err_put: > ida_free(&switchtec_minor_ida, MINOR(stdev->dev.devt)); > put_device(&stdev->dev); > -- > 2.34.1 >
> On Dec 28, 2023, at 3:56 PM, Bjorn Helgaas <helgaas@kernel.org> wrote: > > [+to Daniel, can you take a look? If you like this, I'd like to > squash it into df25461119d9 and credit Christophe since that's not > upstream yet] Thanks very much for fixing this, Christophe. The fix looks correct to me. If it can still fold into the previous change, all the better. Best, Daniel PS: without trying to complicate this thread, does one know idr.[ch] well enough to state whether ida_free() could have gone into stdev_release()? The way the two idr_free calls have been placed looks intentional. But stdev_release would look more obvious + cleaner to me. Was only starting to wonder while reviewing the err_put side of this patch.
On Sun, Dec 24, 2023 at 03:30:01PM +0100, Christophe JAILLET wrote: > The commit in Fixes changed the logic on how resources are released and > introduced a new switchtec_exit_pci() that need to be called explicitly in > order to undo a corresponding switchtec_init_pci(). > > This was done in the remove function, but not in the probe. > > Fix the probe now. > > Fixes: df25461119d9 ("PCI: switchtec: Fix stdev_release() crash after surprise hot remove") > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> Applied to pci/switchtec for v6.9, thanks! > --- > drivers/pci/switch/switchtec.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c > index 1804794d0e68..5a4adf6c04cf 100644 > --- a/drivers/pci/switch/switchtec.c > +++ b/drivers/pci/switch/switchtec.c > @@ -1672,7 +1672,7 @@ static int switchtec_pci_probe(struct pci_dev *pdev, > rc = switchtec_init_isr(stdev); > if (rc) { > dev_err(&stdev->dev, "failed to init isr.\n"); > - goto err_put; > + goto err_exit_pci; > } > > iowrite32(SWITCHTEC_EVENT_CLEAR | > @@ -1693,6 +1693,8 @@ static int switchtec_pci_probe(struct pci_dev *pdev, > > err_devadd: > stdev_kill(stdev); > +err_exit_pci: > + switchtec_exit_pci(stdev); > err_put: > ida_free(&switchtec_minor_ida, MINOR(stdev->dev.devt)); > put_device(&stdev->dev); > -- > 2.34.1 >
diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c index 1804794d0e68..5a4adf6c04cf 100644 --- a/drivers/pci/switch/switchtec.c +++ b/drivers/pci/switch/switchtec.c @@ -1672,7 +1672,7 @@ static int switchtec_pci_probe(struct pci_dev *pdev, rc = switchtec_init_isr(stdev); if (rc) { dev_err(&stdev->dev, "failed to init isr.\n"); - goto err_put; + goto err_exit_pci; } iowrite32(SWITCHTEC_EVENT_CLEAR | @@ -1693,6 +1693,8 @@ static int switchtec_pci_probe(struct pci_dev *pdev, err_devadd: stdev_kill(stdev); +err_exit_pci: + switchtec_exit_pci(stdev); err_put: ida_free(&switchtec_minor_ida, MINOR(stdev->dev.devt)); put_device(&stdev->dev);
The commit in Fixes changed the logic on how resources are released and introduced a new switchtec_exit_pci() that need to be called explicitly in order to undo a corresponding switchtec_init_pci(). This was done in the remove function, but not in the probe. Fix the probe now. Fixes: df25461119d9 ("PCI: switchtec: Fix stdev_release() crash after surprise hot remove") Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> --- drivers/pci/switch/switchtec.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)