* [PATCH][next] media: vivid: fix potential integer overflow on left shift
@ 2019-06-24 21:58 Colin King
2019-06-25 7:48 ` walter harms
0 siblings, 1 reply; 3+ messages in thread
From: Colin King @ 2019-06-24 21:58 UTC (permalink / raw)
To: Hans Verkuil, Mauro Carvalho Chehab, linux-media
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
There is a potential integer overflow when int 2 is left shifted
as this is evaluated using 32 bit arithmetic but is being used in
a context that expects an expression of type s64. Fix this by
shifting 2ULL to avoid a 32 bit overflow.
Addresses-Coverity: ("Unintentional integer overflow")
Fixes: 8a99e9faa131 ("media: vivid: add HDMI (dis)connect RX emulation")
Fixes: 79a792dafac6 ("media: vivid: add HDMI (dis)connect TX emulation")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/media/platform/vivid/vivid-ctrls.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/media/platform/vivid/vivid-ctrls.c b/drivers/media/platform/vivid/vivid-ctrls.c
index 3e916c8befb7..8f340cfd6993 100644
--- a/drivers/media/platform/vivid/vivid-ctrls.c
+++ b/drivers/media/platform/vivid/vivid-ctrls.c
@@ -1634,8 +1634,8 @@ int vivid_create_controls(struct vivid_dev *dev, bool show_ccs_cap,
0, V4L2_DV_RGB_RANGE_AUTO);
dev->ctrl_rx_power_present = v4l2_ctrl_new_std(hdl_vid_cap,
NULL, V4L2_CID_DV_RX_POWER_PRESENT, 0,
- (2 << (dev->num_hdmi_inputs - 1)) - 1, 0,
- (2 << (dev->num_hdmi_inputs - 1)) - 1);
+ (2ULL << (dev->num_hdmi_inputs - 1)) - 1, 0,
+ (2ULL << (dev->num_hdmi_inputs - 1)) - 1);
}
if (dev->num_hdmi_outputs) {
@@ -1653,16 +1653,16 @@ int vivid_create_controls(struct vivid_dev *dev, bool show_ccs_cap,
&vivid_ctrl_display_present, NULL);
dev->ctrl_tx_hotplug = v4l2_ctrl_new_std(hdl_vid_out,
NULL, V4L2_CID_DV_TX_HOTPLUG, 0,
- (2 << (dev->num_hdmi_outputs - 1)) - 1, 0,
- (2 << (dev->num_hdmi_outputs - 1)) - 1);
+ (2ULL << (dev->num_hdmi_outputs - 1)) - 1, 0,
+ (2ULL << (dev->num_hdmi_outputs - 1)) - 1);
dev->ctrl_tx_rxsense = v4l2_ctrl_new_std(hdl_vid_out,
NULL, V4L2_CID_DV_TX_RXSENSE, 0,
- (2 << (dev->num_hdmi_outputs - 1)) - 1, 0,
- (2 << (dev->num_hdmi_outputs - 1)) - 1);
+ (2ULL << (dev->num_hdmi_outputs - 1)) - 1, 0,
+ (2ULL << (dev->num_hdmi_outputs - 1)) - 1);
dev->ctrl_tx_edid_present = v4l2_ctrl_new_std(hdl_vid_out,
NULL, V4L2_CID_DV_TX_EDID_PRESENT, 0,
- (2 << (dev->num_hdmi_outputs - 1)) - 1, 0,
- (2 << (dev->num_hdmi_outputs - 1)) - 1);
+ (2ULL << (dev->num_hdmi_outputs - 1)) - 1, 0,
+ (2ULL << (dev->num_hdmi_outputs - 1)) - 1);
}
if ((dev->has_vid_cap && dev->has_vid_out) ||
(dev->has_vbi_cap && dev->has_vbi_out))
--
2.20.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH][next] media: vivid: fix potential integer overflow on left shift 2019-06-24 21:58 [PATCH][next] media: vivid: fix potential integer overflow on left shift Colin King @ 2019-06-25 7:48 ` walter harms 2019-06-25 7:57 ` Hans Verkuil 0 siblings, 1 reply; 3+ messages in thread From: walter harms @ 2019-06-25 7:48 UTC (permalink / raw) To: Colin King Cc: Hans Verkuil, Mauro Carvalho Chehab, linux-media, kernel-janitors, linux-kernel Am 24.06.2019 23:58, schrieb Colin King: > From: Colin Ian King <colin.king@canonical.com> > > There is a potential integer overflow when int 2 is left shifted > as this is evaluated using 32 bit arithmetic but is being used in > a context that expects an expression of type s64. Fix this by > shifting 2ULL to avoid a 32 bit overflow. > > Addresses-Coverity: ("Unintentional integer overflow") > Fixes: 8a99e9faa131 ("media: vivid: add HDMI (dis)connect RX emulation") > Fixes: 79a792dafac6 ("media: vivid: add HDMI (dis)connect TX emulation") > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > drivers/media/platform/vivid/vivid-ctrls.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/media/platform/vivid/vivid-ctrls.c b/drivers/media/platform/vivid/vivid-ctrls.c > index 3e916c8befb7..8f340cfd6993 100644 > --- a/drivers/media/platform/vivid/vivid-ctrls.c > +++ b/drivers/media/platform/vivid/vivid-ctrls.c > @@ -1634,8 +1634,8 @@ int vivid_create_controls(struct vivid_dev *dev, bool show_ccs_cap, > 0, V4L2_DV_RGB_RANGE_AUTO); > dev->ctrl_rx_power_present = v4l2_ctrl_new_std(hdl_vid_cap, > NULL, V4L2_CID_DV_RX_POWER_PRESENT, 0, > - (2 << (dev->num_hdmi_inputs - 1)) - 1, 0, > - (2 << (dev->num_hdmi_inputs - 1)) - 1); > + (2ULL << (dev->num_hdmi_inputs - 1)) - 1, 0, > + (2ULL << (dev->num_hdmi_inputs - 1)) - 1); > > } > if (dev->num_hdmi_outputs) { > @@ -1653,16 +1653,16 @@ int vivid_create_controls(struct vivid_dev *dev, bool show_ccs_cap, > &vivid_ctrl_display_present, NULL); > dev->ctrl_tx_hotplug = v4l2_ctrl_new_std(hdl_vid_out, > NULL, V4L2_CID_DV_TX_HOTPLUG, 0, > - (2 << (dev->num_hdmi_outputs - 1)) - 1, 0, > - (2 << (dev->num_hdmi_outputs - 1)) - 1); > + (2ULL << (dev->num_hdmi_outputs - 1)) - 1, 0, > + (2ULL << (dev->num_hdmi_outputs - 1)) - 1); > dev->ctrl_tx_rxsense = v4l2_ctrl_new_std(hdl_vid_out, > NULL, V4L2_CID_DV_TX_RXSENSE, 0, > - (2 << (dev->num_hdmi_outputs - 1)) - 1, 0, > - (2 << (dev->num_hdmi_outputs - 1)) - 1); > + (2ULL << (dev->num_hdmi_outputs - 1)) - 1, 0, > + (2ULL << (dev->num_hdmi_outputs - 1)) - 1); > dev->ctrl_tx_edid_present = v4l2_ctrl_new_std(hdl_vid_out, > NULL, V4L2_CID_DV_TX_EDID_PRESENT, 0, > - (2 << (dev->num_hdmi_outputs - 1)) - 1, 0, > - (2 << (dev->num_hdmi_outputs - 1)) - 1); > + (2ULL << (dev->num_hdmi_outputs - 1)) - 1, 0, > + (2ULL << (dev->num_hdmi_outputs - 1)) - 1); > } > if ((dev->has_vid_cap && dev->has_vid_out) || > (dev->has_vbi_cap && dev->has_vbi_out)) To make this more readable for humans, it could help to store (2ULL << (dev->num_hdmi_outputs - 1)) - 1 in an intermediate. like: s64 hdmi=(2ULL << (dev->num_hdmi_outputs - 1)) - 1; dev->ctrl_tx_edid_present = v4l2_ctrl_new_std(hdl_vid_out, NULL, V4L2_CID_DV_TX_EDID_PRESENT, 0, hdmi, 0,hdmi); just my 2 cents, re, wh ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH][next] media: vivid: fix potential integer overflow on left shift 2019-06-25 7:48 ` walter harms @ 2019-06-25 7:57 ` Hans Verkuil 0 siblings, 0 replies; 3+ messages in thread From: Hans Verkuil @ 2019-06-25 7:57 UTC (permalink / raw) To: wharms, Colin King Cc: Mauro Carvalho Chehab, linux-media, kernel-janitors, linux-kernel On 6/25/19 9:48 AM, walter harms wrote: > > > Am 24.06.2019 23:58, schrieb Colin King: >> From: Colin Ian King <colin.king@canonical.com> >> >> There is a potential integer overflow when int 2 is left shifted >> as this is evaluated using 32 bit arithmetic but is being used in >> a context that expects an expression of type s64. Fix this by >> shifting 2ULL to avoid a 32 bit overflow. >> >> Addresses-Coverity: ("Unintentional integer overflow") >> Fixes: 8a99e9faa131 ("media: vivid: add HDMI (dis)connect RX emulation") >> Fixes: 79a792dafac6 ("media: vivid: add HDMI (dis)connect TX emulation") >> Signed-off-by: Colin Ian King <colin.king@canonical.com> >> --- >> drivers/media/platform/vivid/vivid-ctrls.c | 16 ++++++++-------- >> 1 file changed, 8 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/media/platform/vivid/vivid-ctrls.c b/drivers/media/platform/vivid/vivid-ctrls.c >> index 3e916c8befb7..8f340cfd6993 100644 >> --- a/drivers/media/platform/vivid/vivid-ctrls.c >> +++ b/drivers/media/platform/vivid/vivid-ctrls.c >> @@ -1634,8 +1634,8 @@ int vivid_create_controls(struct vivid_dev *dev, bool show_ccs_cap, >> 0, V4L2_DV_RGB_RANGE_AUTO); >> dev->ctrl_rx_power_present = v4l2_ctrl_new_std(hdl_vid_cap, >> NULL, V4L2_CID_DV_RX_POWER_PRESENT, 0, >> - (2 << (dev->num_hdmi_inputs - 1)) - 1, 0, >> - (2 << (dev->num_hdmi_inputs - 1)) - 1); >> + (2ULL << (dev->num_hdmi_inputs - 1)) - 1, 0, >> + (2ULL << (dev->num_hdmi_inputs - 1)) - 1); >> >> } >> if (dev->num_hdmi_outputs) { >> @@ -1653,16 +1653,16 @@ int vivid_create_controls(struct vivid_dev *dev, bool show_ccs_cap, >> &vivid_ctrl_display_present, NULL); >> dev->ctrl_tx_hotplug = v4l2_ctrl_new_std(hdl_vid_out, >> NULL, V4L2_CID_DV_TX_HOTPLUG, 0, >> - (2 << (dev->num_hdmi_outputs - 1)) - 1, 0, >> - (2 << (dev->num_hdmi_outputs - 1)) - 1); >> + (2ULL << (dev->num_hdmi_outputs - 1)) - 1, 0, >> + (2ULL << (dev->num_hdmi_outputs - 1)) - 1); >> dev->ctrl_tx_rxsense = v4l2_ctrl_new_std(hdl_vid_out, >> NULL, V4L2_CID_DV_TX_RXSENSE, 0, >> - (2 << (dev->num_hdmi_outputs - 1)) - 1, 0, >> - (2 << (dev->num_hdmi_outputs - 1)) - 1); >> + (2ULL << (dev->num_hdmi_outputs - 1)) - 1, 0, >> + (2ULL << (dev->num_hdmi_outputs - 1)) - 1); >> dev->ctrl_tx_edid_present = v4l2_ctrl_new_std(hdl_vid_out, >> NULL, V4L2_CID_DV_TX_EDID_PRESENT, 0, >> - (2 << (dev->num_hdmi_outputs - 1)) - 1, 0, >> - (2 << (dev->num_hdmi_outputs - 1)) - 1); >> + (2ULL << (dev->num_hdmi_outputs - 1)) - 1, 0, >> + (2ULL << (dev->num_hdmi_outputs - 1)) - 1); >> } >> if ((dev->has_vid_cap && dev->has_vid_out) || >> (dev->has_vbi_cap && dev->has_vbi_out)) > > > To make this more readable for humans, it could help to store > (2ULL << (dev->num_hdmi_outputs - 1)) - 1 in an intermediate. > like: > s64 hdmi=(2ULL << (dev->num_hdmi_outputs - 1)) - 1; > > dev->ctrl_tx_edid_present = v4l2_ctrl_new_std(hdl_vid_out, > NULL, V4L2_CID_DV_TX_EDID_PRESENT, 0, > hdmi, 0,hdmi); > > > just my 2 cents, I agree. Call it hdmi_output/input_mask, that is a good name for it. Regards, Hans ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-06-25 7:57 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-06-24 21:58 [PATCH][next] media: vivid: fix potential integer overflow on left shift Colin King 2019-06-25 7:48 ` walter harms 2019-06-25 7:57 ` Hans Verkuil
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox