Skip to contentSkip to conversation

Corrected Oops on omap_vout when no manager is connected

PATCH In review Mauro Carvalho Chehab 2 participants 2 messages Sep 15, 2012 – Sep 17, 2012 Media (V4L/DVB)

lore
Share & tools
lore

Mauro Carvalho Chehab

author
#0 lore raw

PATCH 1/1 Corrected Oops on omap_vout when no manager is connected

Prabhakar,

Please review.

Thanks!
Mauro

Forwarded message:

Date: Fri, 24 Aug 2012 17:54:11 +0200
From: Federico Fuga <[email protected]>
To: Mauro Carvalho Chehab <[email protected]>
Cc: [email protected], Federico Fuga <[email protected]>
Subject: [PATCH] Corrected Oops on omap_vout when no manager is connected

If no manager is connected to the vout device, the omapvid_init() function fails. No error condition is checked, and the device is started. Later on, when irq is serviced, a NULL pointer dereference occurs.
Also, the isr routine must be registered only if no error occurs, otherwise the isr triggers without the proper setup, and the kernel oops again. To prevent this, the error condition is checked, and the streamon function exits with error. Also the isr registration call is moved after the setup procedure is completed.

---
 drivers/media/video/omap/omap_vout.c |   14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)
drivers/media/video/omap/omap_vout.c +10 -4
index 15c5f4d..f456587 100644
@@ -650,9 +650,12 @@ static void omap_vout_isr(void *arg, unsigned int irqstatus)
 
 	/* First save the configuration in ovelray structure */
 	ret = omapvid_init(vout, addr);
-	if (ret)
+	if (ret) {
 		printk(KERN_ERR VOUT_NAME
 			"failed to set overlay info\n");
+		goto vout_isr_err;
+	}
+
 	/* Enable the pipeline and set the Go bit */
 	ret = omapvid_apply_changes(vout);
 	if (ret)

@@ -1678,13 +1681,16 @@ static int vidioc_streamon(struct file *file, void *fh, enum v4l2_buf_type i)
 	mask = DISPC_IRQ_VSYNC | DISPC_IRQ_EVSYNC_EVEN | DISPC_IRQ_EVSYNC_ODD
 		| DISPC_IRQ_VSYNC2;
 
-	omap_dispc_register_isr(omap_vout_isr, vout, mask);
-
 	/* First save the configuration in ovelray structure */
 	ret = omapvid_init(vout, addr);
-	if (ret)
+	if (ret) {
 		v4l2_err(&vout->vid_dev->v4l2_dev,
 				"failed to set overlay info\n");
+		goto streamon_err1;
+	}
+
+	omap_dispc_register_isr(omap_vout_isr, vout, mask);
+
 	/* Enable the pipeline and set the Go bit */
 	ret = omapvid_apply_changes(vout);
 	if (ret)
Signature · 11 lines
1.7.9.5

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


-- 
Regards,
Mauro
Reviewed-by on v1 1/1
#1 lore raw
Quoted 16 lines · Mauro Carvalho Chehab

Prabhakar,

Please review.

Thanks!
Mauro

Forwarded message:

Date: Fri, 24 Aug 2012 17:54:11 +0200
From: Federico Fuga <[email protected]>
To: Mauro Carvalho Chehab <[email protected]>
Cc: [email protected], Federico Fuga <[email protected]>
Subject: [PATCH] Corrected Oops on omap_vout when no manager is connected

Quoted 8 lines

If no manager is connected to the vout device, the omapvid_init() function fails. No error condition is checked, and the device is started. Later on, when irq is serviced, a NULL pointer dereference occurs.
Also, the isr routine must be registered only if no error occurs, otherwise the isr triggers without the proper setup, and the kernel oops again. To prevent this, the error condition is checked, and the streamon function exits with error. Also the isr registration call is moved after the setup procedure is completed.

Reviewed-by Prabhakar Lad <[email protected]>

Regards,
--Prabhakar

Quoted 58 lines
---
 drivers/media/video/omap/omap_vout.c |   14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)
drivers/media/video/omap/omap_vout.c +10 -5
index 15c5f4d..f456587 100644
@@ -650,9 +650,12 @@ static void omap_vout_isr(void *arg, unsigned int irqstatus)
 
        /* First save the configuration in ovelray structure */
        ret = omapvid_init(vout, addr);
-       if (ret)
+       if (ret) {
                printk(KERN_ERR VOUT_NAME
                        "failed to set overlay info\n");
+               goto vout_isr_err;
+       }
+
        /* Enable the pipeline and set the Go bit */
        ret = omapvid_apply_changes(vout);
        if (ret)

@@ -1678,13 +1681,16 @@ static int vidioc_streamon(struct file *file, void *fh, enum v4l2_buf_type i)
        mask = DISPC_IRQ_VSYNC | DISPC_IRQ_EVSYNC_EVEN | DISPC_IRQ_EVSYNC_ODD
                | DISPC_IRQ_VSYNC2;
 
-       omap_dispc_register_isr(omap_vout_isr, vout, mask);
-
        /* First save the configuration in ovelray structure */
        ret = omapvid_init(vout, addr);
-       if (ret)
+       if (ret) {
                v4l2_err(&vout->vid_dev->v4l2_dev,
                                "failed to set overlay info\n");
+               goto streamon_err1;
+       }
+
+       omap_dispc_register_isr(omap_vout_isr, vout, mask);
+
        /* Enable the pipeline and set the Go bit */
        ret = omapvid_apply_changes(vout);
        if (ret)
--

1.7.9.5

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

--
Regards,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html