Re: [PING] drm/bochs: replace ioremap with devm_ioremap to avoid immo leak

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

I'm noticed you patch, interesting!

On 2023/3/29 13:26, Gencen Gan wrote:
From: Gan Gecen <gangecen@xxxxxxxxxxx>

Smatch reports:

	drivers/gpu/drm/tiny/bochs.c:290 bochs_hw_init()
	warn: 'bochs->mmio' from ioremap() not released on
	lines: 246,250,254.

In the function bochs_load() that calls bochs_hw_init()
only, if bochs_hw_init(dev) returns -ENODEV(-19), it
will jumps to err_free_dev instead of err_hw_fini, so
bochs->immo won't be freed.

   `mmio`, not `immo`,  you should also fix the typos in you patch's commit title.

We would prefer to replace ioremap with devm_ioremap
to avoid adding lengthy error handling. The function
`devm_ioremap` will automatically release the allocated
resources after use.

Yet, I notice that there is iounmap in bochs_hw_fini() function, does double free will happen?

static void bochs_hw_fini(struct drm_device *dev)
{
    struct bochs_device *bochs = dev->dev_private;
    // ...
    if (bochs->mmio)
        iounmap(bochs->mmio);
    // ...
}


I still advise you free it by adding error handling code, free it manually.

Because still there other ioremap() function in the driver.

But you can choose to heard other reviewer's voice, I can only help to review.

Signed-off-by: Gan Gecen <gangecen@xxxxxxxxxxx>
---
  drivers/gpu/drm/tiny/bochs.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
index 024346054c70..0d7e119a732f 100644
--- a/drivers/gpu/drm/tiny/bochs.c
+++ b/drivers/gpu/drm/tiny/bochs.c
@@ -223,7 +223,7 @@ static int bochs_hw_init(struct drm_device *dev)
  		}
  		ioaddr = pci_resource_start(pdev, 2);
  		iosize = pci_resource_len(pdev, 2);
-		bochs->mmio = ioremap(ioaddr, iosize);
+		bochs->mmio = devm_ioremap(&pdev->dev, ioaddr, iosize);
  		if (bochs->mmio == NULL) {
  			DRM_ERROR("Cannot map mmio region\n");
  			return -ENOMEM;



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux