diff mbox

[media] marvell-ccic: drop free_irq for devm_request_irq allocated irq

Message ID CAPgLHd8hQYqmP=F0MebGuVHHJG8XuSn0Sbe-dJEZoPAEnNV1rA@mail.gmail.com (mailing list archive)
State Accepted, archived
Headers

Commit Message

Wei Yongjun Sept. 24, 2013, 2:35 a.m. UTC 
  From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

irq allocated with devm_request_irq should not be freed using
free_irq, because doing so causes a dangling pointer, and a
subsequent double free.

Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
 drivers/media/platform/marvell-ccic/mmp-driver.c | 1 -
 1 file changed, 1 deletion(-)


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Jonathan Corbet Sept. 25, 2013, 7:11 a.m. UTC | #1 
On Tue, 24 Sep 2013 10:35:50 +0800
Wei Yongjun <weiyj.lk@gmail.com> wrote:

> irq allocated with devm_request_irq should not be freed using
> free_irq, because doing so causes a dangling pointer, and a
> subsequent double free.

Makes sense.

Acked-by: Jonathan Corbet <corbet@lwn.net>

jon
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Libin Yang Sept. 26, 2013, 3:28 a.m. UTC | #2 
Hello Wei Yongjun,

Thanks for finding the issue. It seems the last part code is missed when I submitted the patch. Please see the patch we submitted before in [REVIEW PATCH V4 07/12] [media] marvell-ccic: switch to resource managed allocation and request.

The last part patch is missed:

@@ -474,18 +461,10 @@ out_free:
 static int mmpcam_remove(struct mmp_camera *cam)  {
 	struct mcam_camera *mcam = &cam->mcam;
-	struct mmp_camera_platform_data *pdata;
 
 	mmpcam_remove_device(cam);
-	free_irq(cam->irq, mcam);
 	mccic_shutdown(mcam);
 	mmpcam_power_down(mcam);
-	pdata = cam->pdev->dev.platform_data;
-	gpio_free(pdata->sensor_reset_gpio);
-	gpio_free(pdata->sensor_power_gpio);
-	iounmap(cam->power_regs);
-	iounmap(mcam->regs);
-	kfree(cam);
 	return 0;
 }

It seems there was some mistake when Albert transferring the submit patch task to me. And I did not notice the patch was wrong when submitting the patches. It is my fault. I'm sorry for the inconvenient 

Regards,
Libin 

>-----Original Message-----
>From: linux-media-owner@vger.kernel.org [mailto:linux-media-owner@vger.kernel.org] On
>Behalf Of Jonathan Corbet
>Sent: Wednesday, September 25, 2013 3:12 PM
>To: Wei Yongjun
>Cc: m.chehab@samsung.com; yongjun_wei@trendmicro.com.cn; linux-
>media@vger.kernel.org; Libin Yang
>Subject: Re: [PATCH] [media] marvell-ccic: drop free_irq for devm_request_irq allocated irq
>
>On Tue, 24 Sep 2013 10:35:50 +0800
>Wei Yongjun <weiyj.lk@gmail.com> wrote:
>
>> irq allocated with devm_request_irq should not be freed using
>> free_irq, because doing so causes a dangling pointer, and a subsequent
>> double free.
>
>Makes sense.
>
>Acked-by: Jonathan Corbet <corbet@lwn.net>
>
>jon
>--
>To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message
>to majordomo@vger.kernel.org More majordomo info at  http://vger.kernel.org/majordomo-
>info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c b/drivers/media/platform/marvell-ccic/mmp-driver.c
index b5a19af..3458fa0 100644
--- a/drivers/media/platform/marvell-ccic/mmp-driver.c
+++ b/drivers/media/platform/marvell-ccic/mmp-driver.c
@@ -481,7 +481,6 @@  static int mmpcam_remove(struct mmp_camera *cam)
 	struct mmp_camera_platform_data *pdata;
 
 	mmpcam_remove_device(cam);
-	free_irq(cam->irq, mcam);
 	mccic_shutdown(mcam);
 	mmpcam_power_down(mcam);
 	pdata = cam->pdev->dev.platform_data;