Skip to contentSkip to conversation

V4L/DVB (3420): Added iocls to configure VBI on tvp5150

bug report Dan Carpenter 2 participants 2 messages Feb 19, 2018 Media (V4L/DVB)

lore
Share & tools
lore
#0 lore raw

[ This is obviously ancient code. It's probably fine. I've just been
going through all array overflow warnings recently. - dan ]

Hello Mauro Carvalho Chehab,

The patch 12db56071b47: "V4L/DVB (3420): Added iocls to configure VBI on tvp5150" from Jan 23, 2006, leads to the following static checker warning:

	drivers/media/i2c/tvp5150.c:730 tvp5150_get_vbi()
	error: buffer overflow 'regs' 5 <= 14

drivers/media/i2c/tvp5150.c
699 static int tvp5150_get_vbi(struct v4l2_subdev *sd,
700 const struct i2c_vbi_ram_value *regs, int line)
701 {

   702          struct tvp5150 *decoder = to_tvp5150(sd);
   703          v4l2_std_id std = decoder->norm;
   704          u8 reg;
   705          int pos, type = 0;
   706          int i, ret = 0;
   707  
   708          if (std == V4L2_STD_ALL) {
   709                  dev_err(sd->dev, "VBI can't be configured without knowing number of lines\n");
   710                  return 0;
   711          } else if (std & V4L2_STD_625_50) {
   712                  /* Don't follow NTSC Line number convension */
   713                  line += 3;
   714          }
   715  
   716          if (line < 6 || line > 27)
   717                  return 0;
   718  
   719          reg = ((line - 6) << 1) + TVP5150_LINE_MODE_INI;
   720  
   721          for (i = 0; i <= 1; i++) {
   722                  ret = tvp5150_read(sd, reg + i);
   723                  if (ret < 0) {
   724                          dev_err(sd->dev, "%s: failed with error = %d\n",
   725                                   __func__, ret);
   726                          return 0;
   727                  }
   728                  pos = ret & 0x0f;
   729                  if (pos < 0x0f)
                            ^^^^^^^^^^

Smatch thinks this implies pos can be 0-14.

   730                          type |= regs[pos].type.vbi_type;
                                        ^^^^^^^^^

This array only has 5 elements.

731 }
732
733 return type;
734 }

regards,
dan carpenter

#1 lore raw

Em Mon, 19 Feb 2018 17:27:52 +0300
Dan Carpenter <[email protected]> escreveu:

Quoted 47 lines

[ This is obviously ancient code. It's probably fine. I've just been
going through all array overflow warnings recently. - dan ]

Hello Mauro Carvalho Chehab,

The patch 12db56071b47: "V4L/DVB (3420): Added iocls to configure VBI on tvp5150" from Jan 23, 2006, leads to the following static checker warning:

	drivers/media/i2c/tvp5150.c:730 tvp5150_get_vbi()
	error: buffer overflow 'regs' 5 <= 14

drivers/media/i2c/tvp5150.c
699 static int tvp5150_get_vbi(struct v4l2_subdev *sd,
700 const struct i2c_vbi_ram_value *regs, int line)

   701  {
   702          struct tvp5150 *decoder = to_tvp5150(sd);
   703          v4l2_std_id std = decoder->norm;
   704          u8 reg;
   705          int pos, type = 0;
   706          int i, ret = 0;
   707  
   708          if (std == V4L2_STD_ALL) {
   709                  dev_err(sd->dev, "VBI can't be configured without knowing number of lines\n");
   710                  return 0;
   711          } else if (std & V4L2_STD_625_50) {
   712                  /* Don't follow NTSC Line number convension */
   713                  line += 3;
   714          }
   715  
   716          if (line < 6 || line > 27)
   717                  return 0;
   718  
   719          reg = ((line - 6) << 1) + TVP5150_LINE_MODE_INI;
   720  
   721          for (i = 0; i <= 1; i++) {
   722                  ret = tvp5150_read(sd, reg + i);
   723                  if (ret < 0) {
   724                          dev_err(sd->dev, "%s: failed with error = %d\n",
   725                                   __func__, ret);
   726                          return 0;
   727                  }
   728                  pos = ret & 0x0f;
   729                  if (pos < 0x0f)
                            ^^^^^^^^^^

Smatch thinks this implies pos can be 0-14.

Quoted 8 lines
   730                          type |= regs[pos].type.vbi_type;
                                        ^^^^^^^^^

This array only has 5 elements.

   731          }
   732  
   733          return type;
   734  }

Nice catch! This is indeed a bug. The fact is that no caller drivers currently use the sliced VBI API. Due to that, in practice, this shouldn't be causing any harm.

Yet, better to fix it. IMHO, the better fix would be a patch like the one below.

Regards,

Thanks,
Mauro