* [PATCH v2 1/3] dmaengine: ioatdma: Fix leaking on version mismatch
2024-05-28 6:09 [PATCH v2 0/3] dmaengine: ioatdma: Fix mem leakage series Nikita Shubin via B4 Relay
@ 2024-05-28 6:09 ` Nikita Shubin via B4 Relay
2024-05-28 6:09 ` [PATCH v2 2/3] dmaengine: ioatdma: Fix error path in ioat3_dma_probe() Nikita Shubin via B4 Relay
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Nikita Shubin via B4 Relay @ 2024-05-28 6:09 UTC (permalink / raw)
To: Vinod Koul, Dave Jiang, Logan Gunthorpe
Cc: Andy Shevchenko, Nikita Shubin, dmaengine, linux-kernel, linux,
Nikita Shubin
From: Nikita Shubin <n.shubin@yadro.com>
Fix leaking ioatdma_device if I/OAT version is less than IOAT_VER_3_0.
Fixes: bf453a0a18b2 ("dmaengine: ioat: Support in-use unbind")
Signed-off-by: Nikita Shubin <n.shubin@yadro.com>
---
drivers/dma/ioat/init.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/drivers/dma/ioat/init.c b/drivers/dma/ioat/init.c
index 9c364e92cb82..e76e507ae898 100644
--- a/drivers/dma/ioat/init.c
+++ b/drivers/dma/ioat/init.c
@@ -1350,6 +1350,7 @@ static int ioat_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
void __iomem * const *iomap;
struct device *dev = &pdev->dev;
struct ioatdma_device *device;
+ u8 version;
int err;
err = pcim_enable_device(pdev);
@@ -1363,6 +1364,10 @@ static int ioat_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
if (!iomap)
return -ENOMEM;
+ version = readb(iomap[IOAT_MMIO_BAR] + IOAT_VER_OFFSET);
+ if (version < IOAT_VER_3_0)
+ return -ENODEV;
+
err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
if (err)
return err;
@@ -1373,16 +1378,14 @@ static int ioat_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
pci_set_master(pdev);
pci_set_drvdata(pdev, device);
- device->version = readb(device->reg_base + IOAT_VER_OFFSET);
+ device->version = version;
if (device->version >= IOAT_VER_3_4)
ioat_dca_enabled = 0;
- if (device->version >= IOAT_VER_3_0) {
- if (is_skx_ioat(pdev))
- device->version = IOAT_VER_3_2;
- err = ioat3_dma_probe(device, ioat_dca_enabled);
- } else
- return -ENODEV;
+ if (is_skx_ioat(pdev))
+ device->version = IOAT_VER_3_2;
+
+ err = ioat3_dma_probe(device, ioat_dca_enabled);
if (err) {
dev_err(dev, "Intel(R) I/OAT DMA Engine init failed\n");
return -ENODEV;
--
2.43.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v2 2/3] dmaengine: ioatdma: Fix error path in ioat3_dma_probe()
2024-05-28 6:09 [PATCH v2 0/3] dmaengine: ioatdma: Fix mem leakage series Nikita Shubin via B4 Relay
2024-05-28 6:09 ` [PATCH v2 1/3] dmaengine: ioatdma: Fix leaking on version mismatch Nikita Shubin via B4 Relay
@ 2024-05-28 6:09 ` Nikita Shubin via B4 Relay
2024-05-28 6:09 ` [PATCH v2 3/3] dmaengine: ioatdma: Fix kmemleak in ioat_pci_probe() Nikita Shubin via B4 Relay
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Nikita Shubin via B4 Relay @ 2024-05-28 6:09 UTC (permalink / raw)
To: Vinod Koul, Dave Jiang, Logan Gunthorpe
Cc: Andy Shevchenko, Nikita Shubin, dmaengine, linux-kernel, linux,
Nikita Shubin
From: Nikita Shubin <n.shubin@yadro.com>
Make sure we are disabling interrupts and destroying DMA pool if
pcie_capability_read/write_word() call failed.
Fixes: 511deae0261c ("dmaengine: ioatdma: disable relaxed ordering for ioatdma")
Signed-off-by: Nikita Shubin <n.shubin@yadro.com>
---
drivers/dma/ioat/init.c | 33 +++++++++++++++------------------
1 file changed, 15 insertions(+), 18 deletions(-)
diff --git a/drivers/dma/ioat/init.c b/drivers/dma/ioat/init.c
index e76e507ae898..26964b7c8cf1 100644
--- a/drivers/dma/ioat/init.c
+++ b/drivers/dma/ioat/init.c
@@ -534,18 +534,6 @@ static int ioat_probe(struct ioatdma_device *ioat_dma)
return err;
}
-static int ioat_register(struct ioatdma_device *ioat_dma)
-{
- int err = dma_async_device_register(&ioat_dma->dma_dev);
-
- if (err) {
- ioat_disable_interrupts(ioat_dma);
- dma_pool_destroy(ioat_dma->completion_pool);
- }
-
- return err;
-}
-
static void ioat_dma_remove(struct ioatdma_device *ioat_dma)
{
struct dma_device *dma = &ioat_dma->dma_dev;
@@ -1181,9 +1169,9 @@ static int ioat3_dma_probe(struct ioatdma_device *ioat_dma, int dca)
ioat_chan->reg_base + IOAT_DCACTRL_OFFSET);
}
- err = ioat_register(ioat_dma);
+ err = dma_async_device_register(&ioat_dma->dma_dev);
if (err)
- return err;
+ goto err_disable_interrupts;
ioat_kobject_add(ioat_dma, &ioat_ktype);
@@ -1192,20 +1180,29 @@ static int ioat3_dma_probe(struct ioatdma_device *ioat_dma, int dca)
/* disable relaxed ordering */
err = pcie_capability_read_word(pdev, PCI_EXP_DEVCTL, &val16);
- if (err)
- return pcibios_err_to_errno(err);
+ if (err) {
+ err = pcibios_err_to_errno(err);
+ goto err_disable_interrupts;
+ }
/* clear relaxed ordering enable */
val16 &= ~PCI_EXP_DEVCTL_RELAX_EN;
err = pcie_capability_write_word(pdev, PCI_EXP_DEVCTL, val16);
- if (err)
- return pcibios_err_to_errno(err);
+ if (err) {
+ err = pcibios_err_to_errno(err);
+ goto err_disable_interrupts;
+ }
if (ioat_dma->cap & IOAT_CAP_DPS)
writeb(ioat_pending_level + 1,
ioat_dma->reg_base + IOAT_PREFETCH_LIMIT_OFFSET);
return 0;
+
+err_disable_interrupts:
+ ioat_disable_interrupts(ioat_dma);
+ dma_pool_destroy(ioat_dma->completion_pool);
+ return err;
}
static void ioat_shutdown(struct pci_dev *pdev)
--
2.43.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v2 3/3] dmaengine: ioatdma: Fix kmemleak in ioat_pci_probe()
2024-05-28 6:09 [PATCH v2 0/3] dmaengine: ioatdma: Fix mem leakage series Nikita Shubin via B4 Relay
2024-05-28 6:09 ` [PATCH v2 1/3] dmaengine: ioatdma: Fix leaking on version mismatch Nikita Shubin via B4 Relay
2024-05-28 6:09 ` [PATCH v2 2/3] dmaengine: ioatdma: Fix error path in ioat3_dma_probe() Nikita Shubin via B4 Relay
@ 2024-05-28 6:09 ` Nikita Shubin via B4 Relay
2024-05-28 16:08 ` [PATCH v2 0/3] dmaengine: ioatdma: Fix mem leakage series Dave Jiang
2024-06-11 18:27 ` Vinod Koul
4 siblings, 0 replies; 7+ messages in thread
From: Nikita Shubin via B4 Relay @ 2024-05-28 6:09 UTC (permalink / raw)
To: Vinod Koul, Dave Jiang, Logan Gunthorpe
Cc: Andy Shevchenko, Nikita Shubin, dmaengine, linux-kernel, linux,
Nikita Shubin
From: Nikita Shubin <n.shubin@yadro.com>
If probing fails we end up with leaking ioatdma_device and each
allocated channel.
Following kmemleak easy to reproduce by injecting an error in
ioat_alloc_chan_resources() when doing ioat_dma_self_test().
unreferenced object 0xffff888014ad5800 (size 1024): [..]
[<ffffffff827692ca>] kmemleak_alloc+0x4a/0x80
[<ffffffff81430600>] kmalloc_trace+0x270/0x2f0
[<ffffffffa000b7d1>] ioat_pci_probe+0xc1/0x1c0 [ioatdma]
[..]
repeated for each ioatdma channel:
unreferenced object 0xffff8880148e5c00 (size 512): [..]
[<ffffffff827692ca>] kmemleak_alloc+0x4a/0x80
[<ffffffff81430600>] kmalloc_trace+0x270/0x2f0
[<ffffffffa0009641>] ioat_enumerate_channels+0x101/0x2d0 [ioatdma]
[<ffffffffa000b266>] ioat3_dma_probe+0x4d6/0x970 [ioatdma]
[<ffffffffa000b891>] ioat_pci_probe+0x181/0x1c0 [ioatdma]
[..]
Fixes: bf453a0a18b2 ("dmaengine: ioat: Support in-use unbind")
Signed-off-by: Nikita Shubin <n.shubin@yadro.com>
---
drivers/dma/ioat/init.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/dma/ioat/init.c b/drivers/dma/ioat/init.c
index 26964b7c8cf1..cf688b0c8444 100644
--- a/drivers/dma/ioat/init.c
+++ b/drivers/dma/ioat/init.c
@@ -1347,6 +1347,7 @@ static int ioat_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
void __iomem * const *iomap;
struct device *dev = &pdev->dev;
struct ioatdma_device *device;
+ unsigned int i;
u8 version;
int err;
@@ -1384,6 +1385,9 @@ static int ioat_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
err = ioat3_dma_probe(device, ioat_dca_enabled);
if (err) {
+ for (i = 0; i < IOAT_MAX_CHANS; i++)
+ kfree(device->idx[i]);
+ kfree(device);
dev_err(dev, "Intel(R) I/OAT DMA Engine init failed\n");
return -ENODEV;
}
--
2.43.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2 0/3] dmaengine: ioatdma: Fix mem leakage series
2024-05-28 6:09 [PATCH v2 0/3] dmaengine: ioatdma: Fix mem leakage series Nikita Shubin via B4 Relay
` (2 preceding siblings ...)
2024-05-28 6:09 ` [PATCH v2 3/3] dmaengine: ioatdma: Fix kmemleak in ioat_pci_probe() Nikita Shubin via B4 Relay
@ 2024-05-28 16:08 ` Dave Jiang
2024-05-29 9:34 ` Nikita Shubin
2024-06-11 18:27 ` Vinod Koul
4 siblings, 1 reply; 7+ messages in thread
From: Dave Jiang @ 2024-05-28 16:08 UTC (permalink / raw)
To: n.shubin, Vinod Koul, Logan Gunthorpe
Cc: Andy Shevchenko, Nikita Shubin, dmaengine, linux-kernel, linux
On 5/27/24 11:09 PM, Nikita Shubin via B4 Relay wrote:
> Started with observing leakage in patch 3, investigating revealed much
> more problems in probing error path.
>
> Andy you are always welcome to review if you have a spare time.
>
> Thank you Andy and Markus for your comments.
>
> Signed-off-by: Nikita Shubin <n.shubin@yadro.com>
> ---
> Changes in v2:
> - dmaengine: ioatdma: Fix error path in ioat3_dma_probe():
> Markus:
> - fix typo
>
> - dmaengine: ioatdma: Fix kmemleak in ioat_pci_probe()
> Andy:
> - s/int/unsigned int/
> - fix spelling errors
> - trimmed kmemleak reports
>
> - Link to v1: https://lore.kernel.org/r/20240524-ioatdma-fixes-v1-0-b785f1f7accc@yadro.com
>
> ---
> Nikita Shubin (3):
> dmaengine: ioatdma: Fix leaking on version mismatch
> dmaengine: ioatdma: Fix error path in ioat3_dma_probe()
> dmaengine: ioatdma: Fix kmemleak in ioat_pci_probe()
>
> drivers/dma/ioat/init.c | 54 ++++++++++++++++++++++++++-----------------------
> 1 file changed, 29 insertions(+), 25 deletions(-)
> ---
> base-commit: 6d69b6c12fce479fde7bc06f686212451688a102
> change-id: 20240524-ioatdma-fixes-a8fccda9bd79
Thanks for the fixes.
Reviewed-by: Dave Jiang <dave.jiang@intel.com> for the series
Would be nice if someone wants to move everything to the devm_* management APIs. Would make this a lot less messy. Probably not worth the effort though given how old the driver is and no more devices are being created to use this driver.
>
> Best regards,
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2 0/3] dmaengine: ioatdma: Fix mem leakage series
2024-05-28 16:08 ` [PATCH v2 0/3] dmaengine: ioatdma: Fix mem leakage series Dave Jiang
@ 2024-05-29 9:34 ` Nikita Shubin
0 siblings, 0 replies; 7+ messages in thread
From: Nikita Shubin @ 2024-05-29 9:34 UTC (permalink / raw)
To: Dave Jiang, n.shubin, Vinod Koul, Logan Gunthorpe
Cc: Andy Shevchenko, dmaengine, linux-kernel, linux
Hello Dave!
On Tue, 2024-05-28 at 09:08 -0700, Dave Jiang wrote:
>
>
> On 5/27/24 11:09 PM, Nikita Shubin via B4 Relay wrote:
> > Started with observing leakage in patch 3, investigating revealed
> > much
> > more problems in probing error path.
> >
> > Andy you are always welcome to review if you have a spare time.
> >
> > Thank you Andy and Markus for your comments.
> >
> > Signed-off-by: Nikita Shubin <n.shubin@yadro.com>
> > ---
> > Changes in v2:
> > - dmaengine: ioatdma: Fix error path in ioat3_dma_probe():
> > Markus:
> > - fix typo
> >
> > - dmaengine: ioatdma: Fix kmemleak in ioat_pci_probe()
> > Andy:
> > - s/int/unsigned int/
> > - fix spelling errors
> > - trimmed kmemleak reports
> >
> > - Link to v1:
> > https://lore.kernel.org/r/20240524-ioatdma-fixes-v1-0-b785f1f7accc@yadro.com
> >
> > ---
> > Nikita Shubin (3):
> > dmaengine: ioatdma: Fix leaking on version mismatch
> > dmaengine: ioatdma: Fix error path in ioat3_dma_probe()
> > dmaengine: ioatdma: Fix kmemleak in ioat_pci_probe()
> >
> > drivers/dma/ioat/init.c | 54 ++++++++++++++++++++++++++-----------
> > ------------
> > 1 file changed, 29 insertions(+), 25 deletions(-)
> > ---
> > base-commit: 6d69b6c12fce479fde7bc06f686212451688a102
> > change-id: 20240524-ioatdma-fixes-a8fccda9bd79
>
> Thanks for the fixes.
Glad i could help.
You might find this one useful:
https://patchwork.ozlabs.org/project/qemu-devel/patch/20240524114547.28801-1-nikita.shubin@maquefel.me/
I think sometimes it's much more faster to test something with QEMU
than tinkering with real hardware.
>
> Reviewed-by: Dave Jiang <dave.jiang@intel.com> for the series
>
> Would be nice if someone wants to move everything to the devm_*
> management APIs. Would make this a lot less messy. Probably not worth
> the effort though given how old the driver is and no more devices are
> being created to use this driver.
>
> >
> > Best regards,
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 0/3] dmaengine: ioatdma: Fix mem leakage series
2024-05-28 6:09 [PATCH v2 0/3] dmaengine: ioatdma: Fix mem leakage series Nikita Shubin via B4 Relay
` (3 preceding siblings ...)
2024-05-28 16:08 ` [PATCH v2 0/3] dmaengine: ioatdma: Fix mem leakage series Dave Jiang
@ 2024-06-11 18:27 ` Vinod Koul
4 siblings, 0 replies; 7+ messages in thread
From: Vinod Koul @ 2024-06-11 18:27 UTC (permalink / raw)
To: Dave Jiang, Logan Gunthorpe, Nikita Shubin
Cc: Andy Shevchenko, Nikita Shubin, dmaengine, linux-kernel, linux
On Tue, 28 May 2024 09:09:22 +0300, Nikita Shubin wrote:
> Started with observing leakage in patch 3, investigating revealed much
> more problems in probing error path.
>
> Andy you are always welcome to review if you have a spare time.
>
> Thank you Andy and Markus for your comments.
>
> [...]
Applied, thanks!
[1/3] dmaengine: ioatdma: Fix leaking on version mismatch
commit: 1b11b4ef6bd68591dcaf8423c7d05e794e6aec6f
[2/3] dmaengine: ioatdma: Fix error path in ioat3_dma_probe()
commit: f0dc9fda2e0ee9e01496c2f5aca3a831131fad79
[3/3] dmaengine: ioatdma: Fix kmemleak in ioat_pci_probe()
commit: 29b7cd255f3628e0d65be33a939d8b5bba10aa62
Best regards,
--
Vinod Koul <vkoul@kernel.org>
^ permalink raw reply [flat|nested] 7+ messages in thread