Skip to contentSkip to conversation

saa7146: fix a possible NULL pointer reference in saa7146_set_position

PATCH In review Ethan Zhao 2 participants 3 messages Jan 5, 2014 – Jan 6, 2014

Subsystems (2)
Media (V4L/DVB) SAA7146 VIDEO4LINUX-2 driver
lore
Share & tools
lore

Ethan Zhao

author
#0 lore raw

PATCH 1/1 saa7146: fix a possible NULL pointer reference in saa7146_set_position

Function saa7146_format_by_fourcc() may return NULL, reference of the returned result would cause NULL pointer issue without checking.

Signed-off-by Ethan Zhao <[email protected]>
---
 drivers/media/common/saa7146/saa7146_hlp.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
drivers/media/common/saa7146/saa7146_hlp.c +7 -2
index be746d1..3e23eab 100644
@@ -575,6 +575,7 @@ static void saa7146_set_position(struct saa7146_dev *dev, int w_x, int w_y, int
 	 */
 	u32 base = (u32)(unsigned long)vv->ov_fb.base;
 
+	int which = 1;
 	struct	saa7146_video_dma vdma1;
 
 	/* calculate memory offsets for picture, look if we shall top-down-flip */

@@ -608,10 +609,14 @@ static void saa7146_set_position(struct saa7146_dev *dev, int w_x, int w_y, int
 		vdma1.pitch *= -1;
 	}
 
-	vdma1.base_page = sfmt->swap;
+	if (sfmt)
+		vdma1.base_page = sfmt->swap;
+	else
+		which = 0;
+
 	vdma1.num_line_byte = (vv->standard->v_field<<16)+vv->standard->h_pixels;
 
-	saa7146_write_out_dma(dev, 1, &vdma1);
+	saa7146_write_out_dma(dev, which, &vdma1);
 }
 
 static void saa7146_set_output_format(struct saa7146_dev *dev, unsigned long palette)
Signature · 1 line
1.8.3.4 (Apple Git-47)
#1 lore raw
Quoted 24 lines · Ethan Zhao

Function saa7146_format_by_fourcc() may return NULL, reference of the returned result would cause NULL pointer issue without checking.

Signed-off-by Ethan Zhao <[email protected]>
---
 drivers/media/common/saa7146/saa7146_hlp.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
drivers/media/common/saa7146/saa7146_hlp.c +1 -0
index be746d1..3e23eab 100644
@@ -575,6 +575,7 @@ static void saa7146_set_position(struct saa7146_dev *dev, int w_x, int w_y, int
 	 */
 	u32 base = (u32)(unsigned long)vv->ov_fb.base;
 
+	int which = 1;
 	struct	saa7146_video_dma vdma1;
 
 	/* calculate memory offsets for picture, look if we shall top-down-flip */

@@ -608,10 +609,14 @@ static void saa7146_set_position(struct saa7146_dev *dev, int w_x, int w_y, int
 		vdma1.pitch *= -1;
 	}

drivers/media/common/saa7146/saa7146_hlp.c static void saa7146_set_position(struct saa7146_dev *dev, int w_x, int w_y, int · line 619

Quoted 10 lines
drivers/media/common/saa7146/saa7146_hlp.c +6 -2
-	vdma1.base_page = sfmt->swap;
+	if (sfmt)
+		vdma1.base_page = sfmt->swap;
+	else
+		which = 0;
+
 	vdma1.num_line_byte = (vv->standard->v_field<<16)+vv->standard->h_pixels;
 
-	saa7146_write_out_dma(dev, 1, &vdma1);
+	saa7146_write_out_dma(dev, which, &vdma1);

If saa7146_format_by_fourcc() returns NULL you'll crash much earlier when trying to do

	int depth = sfmt->depth;

I am not familiar with the code by it seems you are papering over the problem. Will we ever get here with unknown pixel format?

Thanks.

Signature · 1 line
Dmitry
author
#2 lore raw

Dmitry,
Yup,thank you point out that, need NULL pointer checking in
calculate_video_dma_grab_packed() , I am not familiar with saa7146 too. wouldn't paper over it, just a fix to pass code review.

Thanks,
Ethan

Quoted 48 lines · Dmitry Torokhov
Quoted 24 lines · Ethan Zhao

Function saa7146_format_by_fourcc() may return NULL, reference of the returned result would cause NULL pointer issue without checking.

Signed-off-by Ethan Zhao <[email protected]>
---
 drivers/media/common/saa7146/saa7146_hlp.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
drivers/media/common/saa7146/saa7146_hlp.c +1 -0
index be746d1..3e23eab 100644
@@ -575,6 +575,7 @@ static void saa7146_set_position(struct saa7146_dev *dev, int w_x, int w_y, int
       */
      u32 base = (u32)(unsigned long)vv->ov_fb.base;
 
+     int which = 1;
      struct  saa7146_video_dma vdma1;
 
      /* calculate memory offsets for picture, look if we shall top-down-flip */

@@ -608,10 +609,14 @@ static void saa7146_set_position(struct saa7146_dev *dev, int w_x, int w_y, int
              vdma1.pitch *= -1;
      }

drivers/media/common/saa7146/saa7146_hlp.c static void saa7146_set_position(struct saa7146_dev *dev, int w_x, int w_y, int · line 619

Quoted 10 lines
drivers/media/common/saa7146/saa7146_hlp.c +6 -2
-     vdma1.base_page = sfmt->swap;
+     if (sfmt)
+             vdma1.base_page = sfmt->swap;
+     else
+             which = 0;
+
      vdma1.num_line_byte = (vv->standard->v_field<<16)+vv->standard->h_pixels;
 
-     saa7146_write_out_dma(dev, 1, &vdma1);
+     saa7146_write_out_dma(dev, which, &vdma1);

If saa7146_format_by_fourcc() returns NULL you'll crash much earlier when trying to do

        int depth = sfmt->depth;

I am not familiar with the code by it seems you are papering over the problem. Will we ever get here with unknown pixel format?

Thanks.

--
Dmitry