Using the managed function simplifies the error handling. After unloading the driver, the PCI device should now get disabled as well.
Signed-off-by: Tian Tao tiantao6@hisilicon.com --- drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c index 02f3bd1..7159018 100644 --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c @@ -329,7 +329,7 @@ static int hibmc_pci_probe(struct pci_dev *pdev, dev->pdev = pdev; pci_set_drvdata(pdev, dev);
- ret = pci_enable_device(pdev); + ret = pcim_enable_device(pdev); if (ret) { drm_err(dev, "failed to enable pci device: %d\n", ret); goto err_free; @@ -338,7 +338,7 @@ static int hibmc_pci_probe(struct pci_dev *pdev, ret = hibmc_load(dev); if (ret) { drm_err(dev, "failed to load hibmc: %d\n", ret); - goto err_disable; + goto err_free; }
ret = drm_dev_register(dev, 0); @@ -354,8 +354,6 @@ static int hibmc_pci_probe(struct pci_dev *pdev,
err_unload: hibmc_unload(dev); -err_disable: - pci_disable_device(pdev); err_free: drm_dev_put(dev);
On Mon, Dec 21, 2020 at 08:45:22AM +0800, Tian Tao wrote:
Using the managed function simplifies the error handling. After unloading the driver, the PCI device should now get disabled as well.
Signed-off-by: Tian Tao tiantao6@hisilicon.com
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c index 02f3bd1..7159018 100644 --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c @@ -329,7 +329,7 @@ static int hibmc_pci_probe(struct pci_dev *pdev, dev->pdev = pdev; pci_set_drvdata(pdev, dev);
- ret = pci_enable_device(pdev);
- ret = pcim_enable_device(pdev); if (ret) { drm_err(dev, "failed to enable pci device: %d\n", ret); goto err_free;
@@ -338,7 +338,7 @@ static int hibmc_pci_probe(struct pci_dev *pdev, ret = hibmc_load(dev); if (ret) { drm_err(dev, "failed to load hibmc: %d\n", ret);
goto err_disable;
goto err_free;}
ret = drm_dev_register(dev, 0);
@@ -354,8 +354,6 @@ static int hibmc_pci_probe(struct pci_dev *pdev,
err_unload: hibmc_unload(dev); -err_disable:
- pci_disable_device(pdev);
err_free: drm_dev_put(dev);
The drm_dev_put here and in hibmc_pci_remove is wrong, devm_drm_dev_alloc takes care of that already. I'm kinda suprised you don't have a refcount underrun already - do you test module unload with KASAN enabled?
The pcim patch looks ok, so Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
Cheers, Daniel
-- 2.7.4
在 2020/12/22 6:02, Daniel Vetter 写道:
On Mon, Dec 21, 2020 at 08:45:22AM +0800, Tian Tao wrote:
Using the managed function simplifies the error handling. After unloading the driver, the PCI device should now get disabled as well.
Signed-off-by: Tian Tao tiantao6@hisilicon.com
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c index 02f3bd1..7159018 100644 --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c @@ -329,7 +329,7 @@ static int hibmc_pci_probe(struct pci_dev *pdev, dev->pdev = pdev; pci_set_drvdata(pdev, dev);
- ret = pci_enable_device(pdev);
- ret = pcim_enable_device(pdev); if (ret) { drm_err(dev, "failed to enable pci device: %d\n", ret); goto err_free;
@@ -338,7 +338,7 @@ static int hibmc_pci_probe(struct pci_dev *pdev, ret = hibmc_load(dev); if (ret) { drm_err(dev, "failed to load hibmc: %d\n", ret);
goto err_disable;
goto err_free;}
ret = drm_dev_register(dev, 0);
@@ -354,8 +354,6 @@ static int hibmc_pci_probe(struct pci_dev *pdev,
err_unload: hibmc_unload(dev); -err_disable:
- pci_disable_device(pdev); err_free: drm_dev_put(dev);
The drm_dev_put here and in hibmc_pci_remove is wrong, devm_drm_dev_alloc takes care of that already. I'm kinda suprised you don't have a refcount underrun already - do you test module unload with KASAN enabled?
Thanks for helping to review the code，and kindly giving me advice。
this problem have been fixed。
c855af2f9c5c60760fd1bed7889a81bc37d2591d drm/hisilicon: Fix use-after-free
The pcim patch looks ok, so Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
Cheers, Daniel
-- 2.7.4
On Tue, Dec 22, 2020 at 1:38 AM tiantao (H) tiantao6@huawei.com wrote:
在 2020/12/22 6:02, Daniel Vetter 写道:
On Mon, Dec 21, 2020 at 08:45:22AM +0800, Tian Tao wrote:
Using the managed function simplifies the error handling. After unloading the driver, the PCI device should now get disabled as well.
Signed-off-by: Tian Tao tiantao6@hisilicon.com
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c index 02f3bd1..7159018 100644 --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c @@ -329,7 +329,7 @@ static int hibmc_pci_probe(struct pci_dev *pdev, dev->pdev = pdev; pci_set_drvdata(pdev, dev);
- ret = pci_enable_device(pdev);
- ret = pcim_enable_device(pdev); if (ret) { drm_err(dev, "failed to enable pci device: %d\n", ret); goto err_free;
@@ -338,7 +338,7 @@ static int hibmc_pci_probe(struct pci_dev *pdev, ret = hibmc_load(dev); if (ret) { drm_err(dev, "failed to load hibmc: %d\n", ret);
goto err_disable;
goto err_free;}
ret = drm_dev_register(dev, 0);
@@ -354,8 +354,6 @@ static int hibmc_pci_probe(struct pci_dev *pdev,
err_unload: hibmc_unload(dev); -err_disable:
- pci_disable_device(pdev); err_free: drm_dev_put(dev);
The drm_dev_put here and in hibmc_pci_remove is wrong, devm_drm_dev_alloc takes care of that already. I'm kinda suprised you don't have a refcount underrun already - do you test module unload with KASAN enabled?
Thanks for helping to review the code，and kindly giving me advice。
this problem have been fixed。
c855af2f9c5c60760fd1bed7889a81bc37d2591d drm/hisilicon: Fix use-after-free
Sorry, I was on an older tree. Note that this fix is incomplete, the drm_dev_put in the error path of hibmc_pci_probe still exists. -Daniel
The pcim patch looks ok, so Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
Cheers, Daniel
-- 2.7.4
在 2020/12/22 16:27, Daniel Vetter 写道:
On Tue, Dec 22, 2020 at 1:38 AM tiantao (H) tiantao6@huawei.com wrote:
在 2020/12/22 6:02, Daniel Vetter 写道:
On Mon, Dec 21, 2020 at 08:45:22AM +0800, Tian Tao wrote:
Using the managed function simplifies the error handling. After unloading the driver, the PCI device should now get disabled as well.
Signed-off-by: Tian Tao tiantao6@hisilicon.com
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c index 02f3bd1..7159018 100644 --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c @@ -329,7 +329,7 @@ static int hibmc_pci_probe(struct pci_dev *pdev, dev->pdev = pdev; pci_set_drvdata(pdev, dev);
- ret = pci_enable_device(pdev);
- ret = pcim_enable_device(pdev); if (ret) { drm_err(dev, "failed to enable pci device: %d\n", ret); goto err_free;
@@ -338,7 +338,7 @@ static int hibmc_pci_probe(struct pci_dev *pdev, ret = hibmc_load(dev); if (ret) { drm_err(dev, "failed to load hibmc: %d\n", ret);
goto err_disable;
goto err_free; } ret = drm_dev_register(dev, 0);@@ -354,8 +354,6 @@ static int hibmc_pci_probe(struct pci_dev *pdev,
err_unload: hibmc_unload(dev); -err_disable:
- pci_disable_device(pdev); err_free: drm_dev_put(dev);
The drm_dev_put here and in hibmc_pci_remove is wrong, devm_drm_dev_alloc takes care of that already. I'm kinda suprised you don't have a refcount underrun already - do you test module unload with KASAN enabled?
Thanks for helping to review the code，and kindly giving me advice。
this problem have been fixed。
c855af2f9c5c60760fd1bed7889a81bc37d2591d drm/hisilicon: Fix use-after-free
Sorry, I was on an older tree. Note that this fix is incomplete, the drm_dev_put in the error path of hibmc_pci_probe still exists. -Daniel
I also found the problem, has raised the patch in the internal review and testing, to confirm that there is no problem, it will be sent out. Thanks for the reminder.
The pcim patch looks ok, so Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
Cheers, Daniel
-- 2.7.4
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Tian Tao -
tiantao (H)