[PATCH] drm/vmwgfx/vmwgfx_drv: Fix an error path in vmw_setup_pci_resources()

[PATCH] drm/vmwgfx/vmwgfx_drv: Fix an error path in vmw_setup_pci_resources()

[Dan Carpenter]

The devm_memremap() function never returns NULL, it returns error
pointers so the test needs to be fixed.  Also we need to call
pci_release_regions() to avoid a memory leak.

Fixes: be4f77ac6884 ("drm/vmwgfx: Cleanup fifo mmio handling")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index eb997f4678de..3f603a6b5f90 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -668,9 +668,10 @@ static int vmw_setup_pci_resources(struct vmw_private *dev,
 				      fifo_size,
 				      MEMREMAP_WB);
 
-	if (unlikely(dev->fifo_mem = NULL)) {
+	if (IS_ERR(dev->fifo_mem)) {
 		DRM_ERROR("Failed mapping FIFO memory.\n");
-		return -ENOMEM;
+		pci_release_regions(pdev);
+		return PTR_ERR(dev->fifo_mem);
 	}
 
 	/*
-- 
2.29.2

[PATCH] drm/vmwgfx/vmwgfx_drv: Fix an error path in vmw_setup_pci_resources()

[Dan Carpenter]

The devm_memremap() function never returns NULL, it returns error
pointers so the test needs to be fixed.  Also we need to call
pci_release_regions() to avoid a memory leak.

Fixes: be4f77ac6884 ("drm/vmwgfx: Cleanup fifo mmio handling")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index eb997f4678de..3f603a6b5f90 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -668,9 +668,10 @@ static int vmw_setup_pci_resources(struct vmw_private *dev,
 				      fifo_size,
 				      MEMREMAP_WB);
 
-	if (unlikely(dev->fifo_mem == NULL)) {
+	if (IS_ERR(dev->fifo_mem)) {
 		DRM_ERROR("Failed mapping FIFO memory.\n");
-		return -ENOMEM;
+		pci_release_regions(pdev);
+		return PTR_ERR(dev->fifo_mem);
 	}
 
 	/*
-- 
2.29.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/vmwgfx/vmwgfx_drv: Fix an error path in vmw_setup_pci_resources()

[Zack Rusin]

DQoNCj4gT24gSmFuIDI1LCAyMDIxLCBhdCAwMzo0NSwgRGFuIENhcnBlbnRlciA8ZGFuLmNhcnBl
bnRlckBvcmFjbGUuY29tPiB3cm90ZToNCj4gDQo+IFRoZSBkZXZtX21lbXJlbWFwKCkgZnVuY3Rp
b24gbmV2ZXIgcmV0dXJucyBOVUxMLCBpdCByZXR1cm5zIGVycm9yDQo+IHBvaW50ZXJzIHNvIHRo
ZSB0ZXN0IG5lZWRzIHRvIGJlIGZpeGVkLiAgQWxzbyB3ZSBuZWVkIHRvIGNhbGwNCj4gcGNpX3Jl
bGVhc2VfcmVnaW9ucygpIHRvIGF2b2lkIGEgbWVtb3J5IGxlYWsuDQo+IA0KPiBGaXhlczogYmU0
Zjc3YWM2ODg0ICgiZHJtL3Ztd2dmeDogQ2xlYW51cCBmaWZvIG1taW8gaGFuZGxpbmciKQ0KPiBT
aWduZWQtb2ZmLWJ5OiBEYW4gQ2FycGVudGVyIDxkYW4uY2FycGVudGVyQG9yYWNsZS5jb20+DQoN
ClRoYW5rcywgRGFuLg0KDQpJIGhhdmUgYSBwYXRjaCBiYXNlZCBvbiB5b3VyIHJlcG9ydCB0aGF0
IGZpeGVzIHRoYXQgdG9vIGJ1dCBpdCBjb21lcyB3aXRoIGEgcmVmYWN0b3JpbmcuIEnigJlkIGJl
IGhhcHB5IHRvIHJlYmFzZSBpdCBvbiB0b3Agb2YgeW91cnMganVzdCB0byBnZXQgeW91cnMgaW4g
YmVmb3JlLiBJZiB5b3XigJlkIGxpa2UgSSBjYW4gcHVzaCB5b3VyIHBhdGNoIHRocm91Z2ggZHJt
LW1pc2MtbmV4dCwgaWYgbm90Og0KDQpSZXZpZXdlZC1ieTogWmFjayBSdXNpbiA8emFja3JAdm13
YXJlLmNvbT4NCg0Keg0KDQoNCg=

Re: [PATCH] drm/vmwgfx/vmwgfx_drv: Fix an error path in vmw_setup_pci_resources()

[Zack Rusin]

> On Jan 25, 2021, at 03:45, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> 
> The devm_memremap() function never returns NULL, it returns error
> pointers so the test needs to be fixed.  Also we need to call
> pci_release_regions() to avoid a memory leak.
> 
> Fixes: be4f77ac6884 ("drm/vmwgfx: Cleanup fifo mmio handling")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Thanks, Dan.

I have a patch based on your report that fixes that too but it comes with a refactoring. I’d be happy to rebase it on top of yours just to get yours in before. If you’d like I can push your patch through drm-misc-next, if not:

Reviewed-by: Zack Rusin <zackr@vmware.com>

z


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/vmwgfx/vmwgfx_drv: Fix an error path in vmw_setup_pci_resources()

[Dan Carpenter]

On Mon, Jan 25, 2021 at 07:13:43PM +0000, Zack Rusin wrote:
> 
> 
> > On Jan 25, 2021, at 03:45, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > 
> > The devm_memremap() function never returns NULL, it returns error
> > pointers so the test needs to be fixed.  Also we need to call
> > pci_release_regions() to avoid a memory leak.
> > 
> > Fixes: be4f77ac6884 ("drm/vmwgfx: Cleanup fifo mmio handling")
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> Thanks, Dan.
> 
> I have a patch based on your report that fixes that too but it comes with a refactoring. I’d be happy to rebase it on top of yours just to get yours in before. If you’d like I can push your patch through drm-misc-next, if not:


No no.  Sorry, just apply your patch and drop mine.  The problem is that
the report I sent was from kbuild and I do my devel work on a different
system so it's harder to track those emails and I forgot.

Looking at it now, the bug report I sent earlier was only for the
pci_release_regions() and not the NULL vs error pointer.  The kbuild
bot is not using full cross function analysis so it can't find those
bugs.  My dev system does much slower builds so it's a tradeoff.

regards,
dan carpenter

Re: [PATCH] drm/vmwgfx/vmwgfx_drv: Fix an error path in vmw_setup_pci_resources()

[Dan Carpenter]

On Mon, Jan 25, 2021 at 07:13:43PM +0000, Zack Rusin wrote:
> 
> 
> > On Jan 25, 2021, at 03:45, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > 
> > The devm_memremap() function never returns NULL, it returns error
> > pointers so the test needs to be fixed.  Also we need to call
> > pci_release_regions() to avoid a memory leak.
> > 
> > Fixes: be4f77ac6884 ("drm/vmwgfx: Cleanup fifo mmio handling")
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> Thanks, Dan.
> 
> I have a patch based on your report that fixes that too but it comes with a refactoring. I’d be happy to rebase it on top of yours just to get yours in before. If you’d like I can push your patch through drm-misc-next, if not:


No no.  Sorry, just apply your patch and drop mine.  The problem is that
the report I sent was from kbuild and I do my devel work on a different
system so it's harder to track those emails and I forgot.

Looking at it now, the bug report I sent earlier was only for the
pci_release_regions() and not the NULL vs error pointer.  The kbuild
bot is not using full cross function analysis so it can't find those
bugs.  My dev system does much slower builds so it's a tradeoff.

regards,
dan carpenter

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel