V4L/DVB (3420): Added iocls to configure VBI on tvp5150
[ 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
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(structv4l2_subdev*sd,
700 const structi2c_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