This 5 patch series addresses a couple of other smatch warnings.
One of the patches seem to be fixing an user-visible bug:
media: uvc: don't do DMA on stack
Basically, input selection at the UVC driver seems broken, as it
is usind DMA on stack, which stopped working on Kernel 4.9.
In practice, the number of affected devices is probably small, as this
affects UVC devices with multiple inputs. The vast majority of UVC
ones have just one input.
Mauro Carvalho Chehab (5):
media: dib8000: rewrite the init prbs logic
media: uvc: don't do DMA on stack
media: v4l2-flash-led-class: drop an useless check
media: ivtv: prevent going past the hw arrays
media: sti: don't copy past the size
drivers/media/dvb-frontends/dib8000.c | 56 ++++++++++-----
drivers/media/pci/ivtv/ivtv-cards.h | 68 +++++++++++++------
drivers/media/pci/ivtv/ivtv-i2c.c | 16 +++--
drivers/media/platform/sti/delta/delta-ipc.c | 3 +-
drivers/media/usb/uvc/uvc_v4l2.c | 10 ++-
drivers/media/usb/uvc/uvcvideo.h | 3 +
.../media/v4l2-core/v4l2-flash-led-class.c | 2 +-
7 files changed, 106 insertions(+), 52 deletions(-)
--
2.31.1
The logic at dib8000_get_init_prbs() has a few issues:
1. the tables used there has an extra unused value at the beginning;
2. the dprintk() message doesn't write the right value when
transmission mode is not 8K;
3. the array overflow validation is done by the callers.
Rewrite the code to fix such issues.
This should also shut up those smatch warnings:
drivers/media/dvb-frontends/dib8000.c:2125 dib8000_get_init_prbs() error: buffer overflow 'lut_prbs_8k' 14 <= 14
drivers/media/dvb-frontends/dib8000.c:2129 dib8000_get_init_prbs() error: buffer overflow 'lut_prbs_2k' 14 <= 14
drivers/media/dvb-frontends/dib8000.c:2131 dib8000_get_init_prbs() error: buffer overflow 'lut_prbs_4k' 14 <= 14
drivers/media/dvb-frontends/dib8000.c:2134 dib8000_get_init_prbs() error: buffer overflow 'lut_prbs_8k' 14 <= 14
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/dvb-frontends/dib8000.c | 56 +++++++++++++++++++--------
1 file changed, 39 insertions(+), 17 deletions(-)
diff --git a/drivers/media/dvb-frontends/dib8000.c b/drivers/media/dvb-frontends/dib8000.c
index 082796534b0a..541f8b9f5f8a 100644
--- a/drivers/media/dvb-frontends/dib8000.c
+++ b/drivers/media/dvb-frontends/dib8000.c
@@ -2107,32 +2107,53 @@ static void dib8000_load_ana_fe_coefs(struct dib8000_state *state, const s16 *an
dib8000_write_word(state, 117 + mode, ana_fe[mode]);
}
-static const u16 lut_prbs_2k[14] = {
- 0, 0x423, 0x009, 0x5C7, 0x7A6, 0x3D8, 0x527, 0x7FF, 0x79B, 0x3D6, 0x3A2, 0x53B, 0x2F4, 0x213
+static const u16 lut_prbs_2k[13] = {
+ 0x423, 0x009, 0x5C7,
+ 0x7A6, 0x3D8, 0x527,
+ 0x7FF, 0x79B, 0x3D6,
+ 0x3A2, 0x53B, 0x2F4,
+ 0x213
};
-static const u16 lut_prbs_4k[14] = {
- 0, 0x208, 0x0C3, 0x7B9, 0x423, 0x5C7, 0x3D8, 0x7FF, 0x3D6, 0x53B, 0x213, 0x029, 0x0D0, 0x48E
+static const u16 lut_prbs_4k[13] = {
+ 0x208, 0x0C3, 0x7B9,
+ 0x423, 0x5C7, 0x3D8,
+ 0x7FF, 0x3D6, 0x53B,
+ 0x213, 0x029, 0x0D0,
+ 0x48E
};
-static const u16 lut_prbs_8k[14] = {
- 0, 0x740, 0x069, 0x7DD, 0x208, 0x7B9, 0x5C7, 0x7FF, 0x53B, 0x029, 0x48E, 0x4C4, 0x367, 0x684
+static const u16 lut_prbs_8k[13] = {
+ 0x740, 0x069, 0x7DD,
+ 0x208, 0x7B9, 0x5C7,
+ 0x7FF, 0x53B, 0x029,
+ 0x48E, 0x4C4, 0x367,
+ 0x684
};
static u16 dib8000_get_init_prbs(struct dib8000_state *state, u16 subchannel)
{
int sub_channel_prbs_group = 0;
+ int prbs_group;
- sub_channel_prbs_group = (subchannel / 3) + 1;
- dprintk("sub_channel_prbs_group = %d , subchannel =%d prbs = 0x%04x\n", sub_channel_prbs_group, subchannel, lut_prbs_8k[sub_channel_prbs_group]);
+ sub_channel_prbs_group = subchannel / 3;
+ if (sub_channel_prbs_group >= ARRAY_SIZE(lut_prbs_2k))
+ return 0;
switch (state->fe[0]->dtv_property_cache.transmission_mode) {
case TRANSMISSION_MODE_2K:
- return lut_prbs_2k[sub_channel_prbs_group];
+ prbs_group = lut_prbs_2k[sub_channel_prbs_group];
+ break;
case TRANSMISSION_MODE_4K:
- return lut_prbs_4k[sub_channel_prbs_group];
+ prbs_group = lut_prbs_4k[sub_channel_prbs_group];
+ break;
default:
case TRANSMISSION_MODE_8K:
- return lut_prbs_8k[sub_channel_prbs_group];
+ prbs_group = lut_prbs_8k[sub_channel_prbs_group];
}
+
+ dprintk("sub_channel_prbs_group = %d , subchannel =%d prbs = 0x%04x\n",
+ sub_channel_prbs_group, subchannel, prbs_group);
+
+ return prbs_group;
}
static void dib8000_set_13seg_channel(struct dib8000_state *state)
@@ -2409,10 +2430,8 @@ static void dib8000_set_isdbt_common_channel(struct dib8000_state *state, u8 seq
/* TSB or ISDBT ? apply it now */
if (c->isdbt_sb_mode) {
dib8000_set_sb_channel(state);
- if (c->isdbt_sb_subchannel < 14)
- init_prbs = dib8000_get_init_prbs(state, c->isdbt_sb_subchannel);
- else
- init_prbs = 0;
+ init_prbs = dib8000_get_init_prbs(state,
+ c->isdbt_sb_subchannel);
} else {
dib8000_set_13seg_channel(state);
init_prbs = 0xfff;
@@ -3004,6 +3023,7 @@ static int dib8000_tune(struct dvb_frontend *fe)
unsigned long *timeout = &state->timeout;
unsigned long now = jiffies;
+ u16 init_prbs;
#ifdef DIB8000_AGC_FREEZE
u16 agc1, agc2;
#endif
@@ -3302,8 +3322,10 @@ static int dib8000_tune(struct dvb_frontend *fe)
break;
case CT_DEMOD_STEP_11: /* 41 : init prbs autosearch */
- if (state->subchannel <= 41) {
- dib8000_set_subchannel_prbs(state, dib8000_get_init_prbs(state, state->subchannel));
+ init_prbs = dib8000_get_init_prbs(state, state->subchannel);
+
+ if (init_prbs) {
+ dib8000_set_subchannel_prbs(state, init_prbs);
*tune_state = CT_DEMOD_STEP_9;
} else {
*tune_state = CT_DEMOD_STOP;
--
2.31.1
As warned by smatch:
drivers/media/pci/ivtv/ivtv-i2c.c:245 ivtv_i2c_register() error: buffer overflow 'hw_devicenames' 21 <= 31
drivers/media/pci/ivtv/ivtv-i2c.c:266 ivtv_i2c_register() error: buffer overflow 'hw_addrs' 21 <= 31
drivers/media/pci/ivtv/ivtv-i2c.c:269 ivtv_i2c_register() error: buffer overflow 'hw_addrs' 21 <= 31
drivers/media/pci/ivtv/ivtv-i2c.c:275 ivtv_i2c_register() error: buffer overflow 'hw_addrs' 21 <= 31
drivers/media/pci/ivtv/ivtv-i2c.c:280 ivtv_i2c_register() error: buffer overflow 'hw_addrs' 21 <= 31
drivers/media/pci/ivtv/ivtv-i2c.c:290 ivtv_i2c_register() error: buffer overflow 'hw_addrs' 21 <= 31
The logic at ivtv_i2c_register() could let buffer overflows at
hw_devicenames and hw_addrs arrays.
This won't happen in practice due to a carefully-contructed
logic, but it is not error-prune.
Change the logic in a way that will make clearer that the
I2C hardware flags will affect the size of those two
arrays, and add an explicit check to avoid buffer overflows.
While here, use the bit macro.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/pci/ivtv/ivtv-cards.h | 68 ++++++++++++++++++++---------
drivers/media/pci/ivtv/ivtv-i2c.c | 16 ++++---
2 files changed, 58 insertions(+), 26 deletions(-)
diff --git a/drivers/media/pci/ivtv/ivtv-cards.h b/drivers/media/pci/ivtv/ivtv-cards.h
index f3e2c5634962..494982e4165d 100644
--- a/drivers/media/pci/ivtv/ivtv-cards.h
+++ b/drivers/media/pci/ivtv/ivtv-cards.h
@@ -78,27 +78,53 @@
#define IVTV_PCI_ID_SONY 0x104d
/* hardware flags, no gaps allowed */
-#define IVTV_HW_CX25840 (1 << 0)
-#define IVTV_HW_SAA7115 (1 << 1)
-#define IVTV_HW_SAA7127 (1 << 2)
-#define IVTV_HW_MSP34XX (1 << 3)
-#define IVTV_HW_TUNER (1 << 4)
-#define IVTV_HW_WM8775 (1 << 5)
-#define IVTV_HW_CS53L32A (1 << 6)
-#define IVTV_HW_TVEEPROM (1 << 7)
-#define IVTV_HW_SAA7114 (1 << 8)
-#define IVTV_HW_UPD64031A (1 << 9)
-#define IVTV_HW_UPD6408X (1 << 10)
-#define IVTV_HW_SAA717X (1 << 11)
-#define IVTV_HW_WM8739 (1 << 12)
-#define IVTV_HW_VP27SMPX (1 << 13)
-#define IVTV_HW_M52790 (1 << 14)
-#define IVTV_HW_GPIO (1 << 15)
-#define IVTV_HW_I2C_IR_RX_AVER (1 << 16)
-#define IVTV_HW_I2C_IR_RX_HAUP_EXT (1 << 17) /* External before internal */
-#define IVTV_HW_I2C_IR_RX_HAUP_INT (1 << 18)
-#define IVTV_HW_Z8F0811_IR_HAUP (1 << 19)
-#define IVTV_HW_I2C_IR_RX_ADAPTEC (1 << 20)
+enum ivtv_hw_bits {
+ IVTV_HW_BIT_CX25840 = 0,
+ IVTV_HW_BIT_SAA7115 = 1,
+ IVTV_HW_BIT_SAA7127 = 2,
+ IVTV_HW_BIT_MSP34XX = 3,
+ IVTV_HW_BIT_TUNER = 4,
+ IVTV_HW_BIT_WM8775 = 5,
+ IVTV_HW_BIT_CS53L32A = 6,
+ IVTV_HW_BIT_TVEEPROM = 7,
+ IVTV_HW_BIT_SAA7114 = 8,
+ IVTV_HW_BIT_UPD64031A = 9,
+ IVTV_HW_BIT_UPD6408X = 10,
+ IVTV_HW_BIT_SAA717X = 11,
+ IVTV_HW_BIT_WM8739 = 12,
+ IVTV_HW_BIT_VP27SMPX = 13,
+ IVTV_HW_BIT_M52790 = 14,
+ IVTV_HW_BIT_GPIO = 15,
+ IVTV_HW_BIT_I2C_IR_RX_AVER = 16,
+ IVTV_HW_BIT_I2C_IR_RX_HAUP_EXT = 17, /* External before internal */
+ IVTV_HW_BIT_I2C_IR_RX_HAUP_INT = 18,
+ IVTV_HW_BIT_Z8F0811_IR_HAUP = 19,
+ IVTV_HW_BIT_I2C_IR_RX_ADAPTEC = 20,
+
+ IVTV_HW_MAX_BITS = 21 /* Should be the last bit + 1 */
+};
+
+#define IVTV_HW_CX25840 BIT(IVTV_HW_BIT_CX25840)
+#define IVTV_HW_SAA7115 BIT(IVTV_HW_BIT_SAA7115)
+#define IVTV_HW_SAA7127 BIT(IVTV_HW_BIT_SAA7127)
+#define IVTV_HW_MSP34XX BIT(IVTV_HW_BIT_MSP34XX)
+#define IVTV_HW_TUNER BIT(IVTV_HW_BIT_TUNER)
+#define IVTV_HW_WM8775 BIT(IVTV_HW_BIT_WM8775)
+#define IVTV_HW_CS53L32A BIT(IVTV_HW_BIT_CS53L32A)
+#define IVTV_HW_TVEEPROM BIT(IVTV_HW_BIT_TVEEPROM)
+#define IVTV_HW_SAA7114 BIT(IVTV_HW_BIT_SAA7114)
+#define IVTV_HW_UPD64031A BIT(IVTV_HW_BIT_UPD64031A)
+#define IVTV_HW_UPD6408X BIT(IVTV_HW_BIT_UPD6408X)
+#define IVTV_HW_SAA717X BIT(IVTV_HW_BIT_SAA717X)
+#define IVTV_HW_WM8739 BIT(IVTV_HW_BIT_WM8739)
+#define IVTV_HW_VP27SMPX BIT(IVTV_HW_BIT_VP27SMPX)
+#define IVTV_HW_M52790 BIT(IVTV_HW_BIT_M52790)
+#define IVTV_HW_GPIO BIT(IVTV_HW_BIT_GPIO)
+#define IVTV_HW_I2C_IR_RX_AVER BIT(IVTV_HW_BIT_I2C_IR_RX_AVER)
+#define IVTV_HW_I2C_IR_RX_HAUP_EXT BIT(IVTV_HW_BIT_I2C_IR_RX_HAUP_EXT)
+#define IVTV_HW_I2C_IR_RX_HAUP_INT BIT(IVTV_HW_BIT_I2C_IR_RX_HAUP_INT)
+#define IVTV_HW_Z8F0811_IR_HAUP BIT(IVTV_HW_BIT_Z8F0811_IR_HAUP)
+#define IVTV_HW_I2C_IR_RX_ADAPTEC BIT(IVTV_HW_BIT_I2C_IR_RX_ADAPTEC)
#define IVTV_HW_SAA711X (IVTV_HW_SAA7115 | IVTV_HW_SAA7114)
diff --git a/drivers/media/pci/ivtv/ivtv-i2c.c b/drivers/media/pci/ivtv/ivtv-i2c.c
index 982045c4eea8..c052c57c6dce 100644
--- a/drivers/media/pci/ivtv/ivtv-i2c.c
+++ b/drivers/media/pci/ivtv/ivtv-i2c.c
@@ -85,7 +85,7 @@
#define IVTV_ADAPTEC_IR_ADDR 0x6b
/* This array should match the IVTV_HW_ defines */
-static const u8 hw_addrs[] = {
+static const u8 hw_addrs[IVTV_HW_MAX_BITS] = {
IVTV_CX25840_I2C_ADDR,
IVTV_SAA7115_I2C_ADDR,
IVTV_SAA7127_I2C_ADDR,
@@ -110,7 +110,7 @@ static const u8 hw_addrs[] = {
};
/* This array should match the IVTV_HW_ defines */
-static const char * const hw_devicenames[] = {
+static const char * const hw_devicenames[IVTV_HW_MAX_BITS] = {
"cx25840",
"saa7115",
"saa7127_auto", /* saa7127 or saa7129 */
@@ -240,10 +240,16 @@ void ivtv_i2c_new_ir_legacy(struct ivtv *itv)
int ivtv_i2c_register(struct ivtv *itv, unsigned idx)
{
- struct v4l2_subdev *sd;
struct i2c_adapter *adap = &itv->i2c_adap;
- const char *type = hw_devicenames[idx];
- u32 hw = 1 << idx;
+ struct v4l2_subdev *sd;
+ const char *type;
+ u32 hw;
+
+ if (idx >= IVTV_HW_MAX_BITS)
+ return -ENODEV;
+
+ type = hw_devicenames[idx];
+ hw = 1 << idx;
if (hw == IVTV_HW_TUNER) {
/* special tuner handling */
--
2.31.1
As warned by smatch:
drivers/media/usb/uvc/uvc_v4l2.c:911 uvc_ioctl_g_input() error: doing dma on the stack (&i)
drivers/media/usb/uvc/uvc_v4l2.c:943 uvc_ioctl_s_input() error: doing dma on the stack (&i)
those two functions call uvc_query_ctrl passing a pointer to
a data at the DMA stack. those are used to send URBs via
usb_control_msg(). Using DMA stack is not supported and should
not work anymore on modern Linux versions.
So, use a temporary buffer, allocated together with
struct uvc_video_chain.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/usb/uvc/uvc_v4l2.c | 10 ++++------
drivers/media/usb/uvc/uvcvideo.h | 3 +++
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index 252136cc885c..e60d4675881a 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -900,7 +900,6 @@ static int uvc_ioctl_g_input(struct file *file, void *fh, unsigned int *input)
struct uvc_fh *handle = fh;
struct uvc_video_chain *chain = handle->chain;
int ret;
- u8 i;
if (chain->selector == NULL ||
(chain->dev->quirks & UVC_QUIRK_IGNORE_SELECTOR_UNIT)) {
@@ -910,11 +909,11 @@ static int uvc_ioctl_g_input(struct file *file, void *fh, unsigned int *input)
ret = uvc_query_ctrl(chain->dev, UVC_GET_CUR, chain->selector->id,
chain->dev->intfnum, UVC_SU_INPUT_SELECT_CONTROL,
- &i, 1);
+ &chain->input, 1);
if (ret < 0)
return ret;
- *input = i - 1;
+ *input = chain->input - 1;
return 0;
}
@@ -923,7 +922,6 @@ static int uvc_ioctl_s_input(struct file *file, void *fh, unsigned int input)
struct uvc_fh *handle = fh;
struct uvc_video_chain *chain = handle->chain;
int ret;
- u32 i;
ret = uvc_acquire_privileges(handle);
if (ret < 0)
@@ -939,10 +937,10 @@ static int uvc_ioctl_s_input(struct file *file, void *fh, unsigned int input)
if (input >= chain->selector->bNrInPins)
return -EINVAL;
- i = input + 1;
+ chain->input = input + 1;
return uvc_query_ctrl(chain->dev, UVC_SET_CUR, chain->selector->id,
chain->dev->intfnum, UVC_SU_INPUT_SELECT_CONTROL,
- &i, 1);
+ &chain->input, 1);
}
static int uvc_ioctl_queryctrl(struct file *file, void *fh,
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index cce5e38133cd..3c0ed90d6912 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -475,6 +475,9 @@ struct uvc_video_chain {
struct mutex ctrl_mutex; /* Protects ctrl.info */
struct v4l2_prio_state prio; /* V4L2 priority state */
+
+ u8 input; /* buffer for set/get input */
+
u32 caps; /* V4L2 chain-wide caps */
};
--
2.31.1
Hi Mauro,
Thank you for the patch.
On Mon, Jun 21, 2021 at 01:56:46PM +0200, Mauro Carvalho Chehab wrote:
> As warned by smatch:
> drivers/media/usb/uvc/uvc_v4l2.c:911 uvc_ioctl_g_input() error: doing dma on the stack (&i)
> drivers/media/usb/uvc/uvc_v4l2.c:943 uvc_ioctl_s_input() error: doing dma on the stack (&i)
>
> those two functions call uvc_query_ctrl passing a pointer to
> a data at the DMA stack. those are used to send URBs via
> usb_control_msg(). Using DMA stack is not supported and should
> not work anymore on modern Linux versions.
>
> So, use a temporary buffer, allocated together with
> struct uvc_video_chain.
DMA in a memory location that may share a cache line with something else
isn't a great idea either. The buffer should be kmalloc()ed in
uvc_ioctl_g_input() and uvc_ioctl_s_input().
> Signed-off-by: Mauro Carvalho Chehab <[email protected]>
> ---
> drivers/media/usb/uvc/uvc_v4l2.c | 10 ++++------
> drivers/media/usb/uvc/uvcvideo.h | 3 +++
> 2 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
> index 252136cc885c..e60d4675881a 100644
> --- a/drivers/media/usb/uvc/uvc_v4l2.c
> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> @@ -900,7 +900,6 @@ static int uvc_ioctl_g_input(struct file *file, void *fh, unsigned int *input)
> struct uvc_fh *handle = fh;
> struct uvc_video_chain *chain = handle->chain;
> int ret;
> - u8 i;
>
> if (chain->selector == NULL ||
> (chain->dev->quirks & UVC_QUIRK_IGNORE_SELECTOR_UNIT)) {
> @@ -910,11 +909,11 @@ static int uvc_ioctl_g_input(struct file *file, void *fh, unsigned int *input)
>
> ret = uvc_query_ctrl(chain->dev, UVC_GET_CUR, chain->selector->id,
> chain->dev->intfnum, UVC_SU_INPUT_SELECT_CONTROL,
> - &i, 1);
> + &chain->input, 1);
> if (ret < 0)
> return ret;
>
> - *input = i - 1;
> + *input = chain->input - 1;
> return 0;
> }
>
> @@ -923,7 +922,6 @@ static int uvc_ioctl_s_input(struct file *file, void *fh, unsigned int input)
> struct uvc_fh *handle = fh;
> struct uvc_video_chain *chain = handle->chain;
> int ret;
> - u32 i;
>
> ret = uvc_acquire_privileges(handle);
> if (ret < 0)
> @@ -939,10 +937,10 @@ static int uvc_ioctl_s_input(struct file *file, void *fh, unsigned int input)
> if (input >= chain->selector->bNrInPins)
> return -EINVAL;
>
> - i = input + 1;
> + chain->input = input + 1;
> return uvc_query_ctrl(chain->dev, UVC_SET_CUR, chain->selector->id,
> chain->dev->intfnum, UVC_SU_INPUT_SELECT_CONTROL,
> - &i, 1);
> + &chain->input, 1);
> }
>
> static int uvc_ioctl_queryctrl(struct file *file, void *fh,
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index cce5e38133cd..3c0ed90d6912 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -475,6 +475,9 @@ struct uvc_video_chain {
> struct mutex ctrl_mutex; /* Protects ctrl.info */
>
> struct v4l2_prio_state prio; /* V4L2 priority state */
> +
> + u8 input; /* buffer for set/get input */
> +
> u32 caps; /* V4L2 chain-wide caps */
> };
>
--
Regards,
Laurent Pinchart
On 21/06/2021 13:56, Mauro Carvalho Chehab wrote:
> As warned by smatch:
>
> drivers/media/pci/ivtv/ivtv-i2c.c:245 ivtv_i2c_register() error: buffer overflow 'hw_devicenames' 21 <= 31
> drivers/media/pci/ivtv/ivtv-i2c.c:266 ivtv_i2c_register() error: buffer overflow 'hw_addrs' 21 <= 31
> drivers/media/pci/ivtv/ivtv-i2c.c:269 ivtv_i2c_register() error: buffer overflow 'hw_addrs' 21 <= 31
> drivers/media/pci/ivtv/ivtv-i2c.c:275 ivtv_i2c_register() error: buffer overflow 'hw_addrs' 21 <= 31
> drivers/media/pci/ivtv/ivtv-i2c.c:280 ivtv_i2c_register() error: buffer overflow 'hw_addrs' 21 <= 31
> drivers/media/pci/ivtv/ivtv-i2c.c:290 ivtv_i2c_register() error: buffer overflow 'hw_addrs' 21 <= 31
>
> The logic at ivtv_i2c_register() could let buffer overflows at
> hw_devicenames and hw_addrs arrays.
>
> This won't happen in practice due to a carefully-contructed
> logic, but it is not error-prune.
>
> Change the logic in a way that will make clearer that the
> I2C hardware flags will affect the size of those two
> arrays, and add an explicit check to avoid buffer overflows.
>
> While here, use the bit macro.
>
> Signed-off-by: Mauro Carvalho Chehab <[email protected]>
> ---
> drivers/media/pci/ivtv/ivtv-cards.h | 68 ++++++++++++++++++++---------
> drivers/media/pci/ivtv/ivtv-i2c.c | 16 ++++---
> 2 files changed, 58 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/media/pci/ivtv/ivtv-cards.h b/drivers/media/pci/ivtv/ivtv-cards.h
> index f3e2c5634962..494982e4165d 100644
> --- a/drivers/media/pci/ivtv/ivtv-cards.h
> +++ b/drivers/media/pci/ivtv/ivtv-cards.h
> @@ -78,27 +78,53 @@
> #define IVTV_PCI_ID_SONY 0x104d
>
> /* hardware flags, no gaps allowed */
> -#define IVTV_HW_CX25840 (1 << 0)
> -#define IVTV_HW_SAA7115 (1 << 1)
> -#define IVTV_HW_SAA7127 (1 << 2)
> -#define IVTV_HW_MSP34XX (1 << 3)
> -#define IVTV_HW_TUNER (1 << 4)
> -#define IVTV_HW_WM8775 (1 << 5)
> -#define IVTV_HW_CS53L32A (1 << 6)
> -#define IVTV_HW_TVEEPROM (1 << 7)
> -#define IVTV_HW_SAA7114 (1 << 8)
> -#define IVTV_HW_UPD64031A (1 << 9)
> -#define IVTV_HW_UPD6408X (1 << 10)
> -#define IVTV_HW_SAA717X (1 << 11)
> -#define IVTV_HW_WM8739 (1 << 12)
> -#define IVTV_HW_VP27SMPX (1 << 13)
> -#define IVTV_HW_M52790 (1 << 14)
> -#define IVTV_HW_GPIO (1 << 15)
> -#define IVTV_HW_I2C_IR_RX_AVER (1 << 16)
> -#define IVTV_HW_I2C_IR_RX_HAUP_EXT (1 << 17) /* External before internal */
> -#define IVTV_HW_I2C_IR_RX_HAUP_INT (1 << 18)
> -#define IVTV_HW_Z8F0811_IR_HAUP (1 << 19)
> -#define IVTV_HW_I2C_IR_RX_ADAPTEC (1 << 20)
> +enum ivtv_hw_bits {
> + IVTV_HW_BIT_CX25840 = 0,
> + IVTV_HW_BIT_SAA7115 = 1,
> + IVTV_HW_BIT_SAA7127 = 2,
> + IVTV_HW_BIT_MSP34XX = 3,
> + IVTV_HW_BIT_TUNER = 4,
> + IVTV_HW_BIT_WM8775 = 5,
> + IVTV_HW_BIT_CS53L32A = 6,
> + IVTV_HW_BIT_TVEEPROM = 7,
> + IVTV_HW_BIT_SAA7114 = 8,
> + IVTV_HW_BIT_UPD64031A = 9,
> + IVTV_HW_BIT_UPD6408X = 10,
> + IVTV_HW_BIT_SAA717X = 11,
> + IVTV_HW_BIT_WM8739 = 12,
> + IVTV_HW_BIT_VP27SMPX = 13,
> + IVTV_HW_BIT_M52790 = 14,
> + IVTV_HW_BIT_GPIO = 15,
> + IVTV_HW_BIT_I2C_IR_RX_AVER = 16,
> + IVTV_HW_BIT_I2C_IR_RX_HAUP_EXT = 17, /* External before internal */
> + IVTV_HW_BIT_I2C_IR_RX_HAUP_INT = 18,
> + IVTV_HW_BIT_Z8F0811_IR_HAUP = 19,
> + IVTV_HW_BIT_I2C_IR_RX_ADAPTEC = 20,
> +
> + IVTV_HW_MAX_BITS = 21 /* Should be the last bit + 1 */
It's an enum, so you can drop the '= nr' bit.
Other than that it looks OK to me.
Regards,
Hans
> +};
> +
> +#define IVTV_HW_CX25840 BIT(IVTV_HW_BIT_CX25840)
> +#define IVTV_HW_SAA7115 BIT(IVTV_HW_BIT_SAA7115)
> +#define IVTV_HW_SAA7127 BIT(IVTV_HW_BIT_SAA7127)
> +#define IVTV_HW_MSP34XX BIT(IVTV_HW_BIT_MSP34XX)
> +#define IVTV_HW_TUNER BIT(IVTV_HW_BIT_TUNER)
> +#define IVTV_HW_WM8775 BIT(IVTV_HW_BIT_WM8775)
> +#define IVTV_HW_CS53L32A BIT(IVTV_HW_BIT_CS53L32A)
> +#define IVTV_HW_TVEEPROM BIT(IVTV_HW_BIT_TVEEPROM)
> +#define IVTV_HW_SAA7114 BIT(IVTV_HW_BIT_SAA7114)
> +#define IVTV_HW_UPD64031A BIT(IVTV_HW_BIT_UPD64031A)
> +#define IVTV_HW_UPD6408X BIT(IVTV_HW_BIT_UPD6408X)
> +#define IVTV_HW_SAA717X BIT(IVTV_HW_BIT_SAA717X)
> +#define IVTV_HW_WM8739 BIT(IVTV_HW_BIT_WM8739)
> +#define IVTV_HW_VP27SMPX BIT(IVTV_HW_BIT_VP27SMPX)
> +#define IVTV_HW_M52790 BIT(IVTV_HW_BIT_M52790)
> +#define IVTV_HW_GPIO BIT(IVTV_HW_BIT_GPIO)
> +#define IVTV_HW_I2C_IR_RX_AVER BIT(IVTV_HW_BIT_I2C_IR_RX_AVER)
> +#define IVTV_HW_I2C_IR_RX_HAUP_EXT BIT(IVTV_HW_BIT_I2C_IR_RX_HAUP_EXT)
> +#define IVTV_HW_I2C_IR_RX_HAUP_INT BIT(IVTV_HW_BIT_I2C_IR_RX_HAUP_INT)
> +#define IVTV_HW_Z8F0811_IR_HAUP BIT(IVTV_HW_BIT_Z8F0811_IR_HAUP)
> +#define IVTV_HW_I2C_IR_RX_ADAPTEC BIT(IVTV_HW_BIT_I2C_IR_RX_ADAPTEC)
>
> #define IVTV_HW_SAA711X (IVTV_HW_SAA7115 | IVTV_HW_SAA7114)
>
> diff --git a/drivers/media/pci/ivtv/ivtv-i2c.c b/drivers/media/pci/ivtv/ivtv-i2c.c
> index 982045c4eea8..c052c57c6dce 100644
> --- a/drivers/media/pci/ivtv/ivtv-i2c.c
> +++ b/drivers/media/pci/ivtv/ivtv-i2c.c
> @@ -85,7 +85,7 @@
> #define IVTV_ADAPTEC_IR_ADDR 0x6b
>
> /* This array should match the IVTV_HW_ defines */
> -static const u8 hw_addrs[] = {
> +static const u8 hw_addrs[IVTV_HW_MAX_BITS] = {
> IVTV_CX25840_I2C_ADDR,
> IVTV_SAA7115_I2C_ADDR,
> IVTV_SAA7127_I2C_ADDR,
> @@ -110,7 +110,7 @@ static const u8 hw_addrs[] = {
> };
>
> /* This array should match the IVTV_HW_ defines */
> -static const char * const hw_devicenames[] = {
> +static const char * const hw_devicenames[IVTV_HW_MAX_BITS] = {
> "cx25840",
> "saa7115",
> "saa7127_auto", /* saa7127 or saa7129 */
> @@ -240,10 +240,16 @@ void ivtv_i2c_new_ir_legacy(struct ivtv *itv)
>
> int ivtv_i2c_register(struct ivtv *itv, unsigned idx)
> {
> - struct v4l2_subdev *sd;
> struct i2c_adapter *adap = &itv->i2c_adap;
> - const char *type = hw_devicenames[idx];
> - u32 hw = 1 << idx;
> + struct v4l2_subdev *sd;
> + const char *type;
> + u32 hw;
> +
> + if (idx >= IVTV_HW_MAX_BITS)
> + return -ENODEV;
> +
> + type = hw_devicenames[idx];
> + hw = 1 << idx;
>
> if (hw == IVTV_HW_TUNER) {
> /* special tuner handling */
>