* [PATCH] [media] s3c-camif: array underflow in __camif_subdev_try_format()
@ 2018-01-22 10:37 Dan Carpenter
2018-01-22 20:50 ` Sylwester Nawrocki
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2018-01-22 10:37 UTC (permalink / raw)
To: Sylwester Nawrocki
Cc: Mauro Carvalho Chehab, linux-media, linux-samsung-soc,
kernel-janitors
The while loop is a post op, "while (i-- >= 0)" so the last iteration
will read camif_mbus_formats[-1] and then the loop will exit with "i"
set to -2 and so we do: "mf->code = camif_mbus_formats[-2];".
I've changed it to a pre-op, I've added a check to ensure we found the
right format and I've removed the "mf->code = camif_mbus_formats[i];"
because that is a no-op anyway.
Fixes: babde1c243b2 ("[media] V4L: Add driver for S3C24XX/S3C64XX SoC series camera interface")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/media/platform/s3c-camif/camif-capture.c b/drivers/media/platform/s3c-camif/camif-capture.c
index 437395a61065..012f4b389c55 100644
--- a/drivers/media/platform/s3c-camif/camif-capture.c
+++ b/drivers/media/platform/s3c-camif/camif-capture.c
@@ -1261,11 +1261,11 @@ static void __camif_subdev_try_format(struct camif_dev *camif,
/* FIXME: constraints against codec or preview path ? */
pix_lim = &variant->vp_pix_limits[VP_CODEC];
- while (i-- >= 0)
+ while (--i >= 0)
if (camif_mbus_formats[i] = mf->code)
break;
-
- mf->code = camif_mbus_formats[i];
+ if (i < 0)
+ return;
if (pad = CAMIF_SD_PAD_SINK) {
v4l_bound_align_image(&mf->width, 8, CAMIF_MAX_PIX_WIDTH,
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] [media] s3c-camif: array underflow in __camif_subdev_try_format()
2018-01-22 10:37 [PATCH] [media] s3c-camif: array underflow in __camif_subdev_try_format() Dan Carpenter
@ 2018-01-22 20:50 ` Sylwester Nawrocki
2018-01-24 8:13 ` Dan Carpenter
0 siblings, 1 reply; 4+ messages in thread
From: Sylwester Nawrocki @ 2018-01-22 20:50 UTC (permalink / raw)
To: Dan Carpenter
Cc: Mauro Carvalho Chehab, linux-media, linux-samsung-soc,
kernel-janitors, Arnd Bergmann
On 01/22/2018 11:37 AM, Dan Carpenter wrote:
> The while loop is a post op, "while (i-- >= 0)" so the last iteration
> will read camif_mbus_formats[-1] and then the loop will exit with "i"
> set to -2 and so we do: "mf->code = camif_mbus_formats[-2];".
>
> I've changed it to a pre-op, I've added a check to ensure we found the
> right format and I've removed the "mf->code = camif_mbus_formats[i];"
> because that is a no-op anyway.
>
> Fixes: babde1c243b2 ("[media] V4L: Add driver for S3C24XX/S3C64XX SoC series camera interface")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/drivers/media/platform/s3c-camif/camif-capture.c b/drivers/media/platform/s3c-camif/camif-capture.c
> index 437395a61065..012f4b389c55 100644
> --- a/drivers/media/platform/s3c-camif/camif-capture.c
> +++ b/drivers/media/platform/s3c-camif/camif-capture.c
> @@ -1261,11 +1261,11 @@ static void __camif_subdev_try_format(struct camif_dev *camif,
> /* FIXME: constraints against codec or preview path ? */
> pix_lim = &variant->vp_pix_limits[VP_CODEC];
>
> - while (i-- >= 0)
> + while (--i >= 0)
> if (camif_mbus_formats[i] = mf->code)
> break;
> -
> - mf->code = camif_mbus_formats[i];
> + if (i < 0)
> + return;
Thanks for the patch Dan. mf->width needs to be aligned by this try_format
function so we shouldn't return here. Also it needs to be ensured mf->code
is set to one of the supported values when this function returns. Sorry,
the current code really doesn't give a clue what was intended.
There is already queued a patch from Arnd [1] addressing the issues you
have found.
> if (pad = CAMIF_SD_PAD_SINK) {
> v4l_bound_align_image(&mf->width, 8, CAMIF_MAX_PIX_WIDTH,
>
[1] https://patchwork.linuxtv.org/patch/46508
--
Regards,
Sylwester
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] [media] s3c-camif: array underflow in __camif_subdev_try_format()
2018-01-22 20:50 ` Sylwester Nawrocki
@ 2018-01-24 8:13 ` Dan Carpenter
2018-01-24 8:43 ` Arnd Bergmann
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2018-01-24 8:13 UTC (permalink / raw)
To: Sylwester Nawrocki
Cc: Mauro Carvalho Chehab, linux-media, linux-samsung-soc,
kernel-janitors, Arnd Bergmann
On Mon, Jan 22, 2018 at 09:50:04PM +0100, Sylwester Nawrocki wrote:
> On 01/22/2018 11:37 AM, Dan Carpenter wrote:
> > --- a/drivers/media/platform/s3c-camif/camif-capture.c
> > +++ b/drivers/media/platform/s3c-camif/camif-capture.c
> > @@ -1261,11 +1261,11 @@ static void __camif_subdev_try_format(struct camif_dev *camif,
> > /* FIXME: constraints against codec or preview path ? */
> > pix_lim = &variant->vp_pix_limits[VP_CODEC];
> >
> > - while (i-- >= 0)
> > + while (--i >= 0)
> > if (camif_mbus_formats[i] = mf->code)
> > break;
> > -
> > - mf->code = camif_mbus_formats[i];
> > + if (i < 0)
> > + return;
>
> Thanks for the patch Dan. mf->width needs to be aligned by this try_format
> function so we shouldn't return here. Also it needs to be ensured mf->code
> is set to one of the supported values when this function returns. Sorry,
> the current code really doesn't give a clue what was intended.
>
> There is already queued a patch from Arnd [1] addressing the issues you
> have found.
>
> > if (pad = CAMIF_SD_PAD_SINK) {
> > v4l_bound_align_image(&mf->width, 8, CAMIF_MAX_PIX_WIDTH,
> >
>
> [1] https://patchwork.linuxtv.org/patch/46508
>
Hey Arnd,
I happened to be looking at the same bugs but using Smatch. Did you get
these two bugs as well?
drivers/scsi/sym53c8xx_2/sym_hipd.c:549 sym_getsync() error: iterator underflow 'div_10M' (-1)-255
drivers/media/i2c/sr030pc30.c:522 try_fmt() error: iterator underflow 'sr030pc30_formats' (-1)-4
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] [media] s3c-camif: array underflow in __camif_subdev_try_format()
2018-01-24 8:13 ` Dan Carpenter
@ 2018-01-24 8:43 ` Arnd Bergmann
0 siblings, 0 replies; 4+ messages in thread
From: Arnd Bergmann @ 2018-01-24 8:43 UTC (permalink / raw)
To: Dan Carpenter
Cc: Sylwester Nawrocki, Mauro Carvalho Chehab,
Linux Media Mailing List,
moderated list:ARM/SAMSUNG EXYNOS ARM ARCHITECTURES,
kernel-janitors
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 4009 bytes --]
On Wed, Jan 24, 2018 at 9:13 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> On Mon, Jan 22, 2018 at 09:50:04PM +0100, Sylwester Nawrocki wrote:
>> On 01/22/2018 11:37 AM, Dan Carpenter wrote:
> I happened to be looking at the same bugs but using Smatch. Did you get
> these two bugs as well?
>
> drivers/scsi/sym53c8xx_2/sym_hipd.c:549 sym_getsync() error: iterator underflow 'div_10M' (-1)-255
> drivers/media/i2c/sr030pc30.c:522 try_fmt() error: iterator underflow 'sr030pc30_formats' (-1)-4
I don't recall seeing those two, here is a list of array-bounds
warnings I had in the past
months, mostly while building with gcc-7.2:
/git/arm-soc/arch/arm/mach-vexpress/spc.c:431:38: warning: array
subscript is below array bounds [-Warray-bounds]
/git/arm-soc/arch/x86/include/asm/string_32.h:70:16: error: array
subscript is above array bounds [-Werror=array-bounds]
/git/arm-soc/arch/x86/include/asm/uaccess_64.h:143:20: error: array
subscript is above array bounds [-Werror=array-bounds]
/git/arm-soc/drivers/cpufreq/arm_big_little.c:201:24: warning: array
subscript is above array bounds [-Warray-bounds]
/git/arm-soc/drivers/cpufreq/arm_big_little.c:325:16: warning: array
subscript is above array bounds [-Warray-bounds]
/git/arm-soc/drivers/cpufreq/arm_big_little.c:440:39: warning: array
subscript is above array bounds [-Warray-bounds]
/git/arm-soc/drivers/dma/sh/rcar-dmac.c:876:29: error: array subscript
is above array bounds [-Werror=array-bounds]
/git/arm-soc/drivers/isdn/hardware/eicon/message.c:11302:54: error:
array subscript is above array bounds [-Werror=array-bounds]
/git/arm-soc/drivers/net/ethernet/intel/igb/igb_ptp.c:367: error:
array subscript is below array bounds
/git/arm-soc/drivers/net/ethernet/intel/igb/igb_ptp.c:455: error:
array subscript is below array bounds
/git/arm-soc/drivers/scsi/qla2xxx/qla_gs.c:1398:7: error: array
subscript is above array bounds [-Werror=array-bounds]
/git/arm-soc/drivers/scsi/qla2xxx/qla_gs.c:2279:7: error: array
subscript is above array bounds [-Werror=array-bounds]
/git/arm-soc/drivers/scsi/sym53c416.c:565:58: error: array subscript
is above array bounds [-Werror=array-bounds]
/git/arm-soc/drivers/staging/lustre/lustre/ptlrpc/ptlrpcd.c:492:14:
error: array subscript -1 is below array bounds of 'struct
ptlrpcd_ctl[]' [-Werror=array-bounds]
/git/arm-soc/fs/f2fs/segment.c:3044:5: error: array subscript is below
array bounds [-Werror=array-bounds]
/git/arm-soc/include/linux/compiler.h:253:20: error: array subscript
is above array bounds [-Werror=array-bounds]
/git/arm-soc/include/linux/dynamic_debug.h:86:20: warning: array
subscript is above array bounds [-Warray-bounds]
/git/arm-soc/include/sound/pcm.h:919:9: error: array subscript is
above array bounds [-Werror=array-bounds]
/git/arm-soc/kernel/bpf/verifier.c:4320:29: error: array subscript is
above array bounds [-Werror=array-bounds]
/git/arm-soc/kernel/rcu/tree.c:3332:13: warning: array subscript is
above array bounds [-Warray-bounds]
/git/arm-soc/net/ipv4/tcp_output.c:2129:40: error: array subscript is
below array bounds [-Werror=array-bounds]
/git/arm-soc/net/ipv4/tcp_output.c:2207:40: error: array subscript is
below array bounds [-Werror=array-bounds]
/git/arm-soc/net/rxrpc/ar-connection.c:589:16: warning: array
subscript is above array bounds [-Warray-bounds]
/git/arm-soc/sound/soc/sh/rcar/cmd.c:88:14: error: array subscript is
below array bounds [-Werror=array-bounds]
/git/arm-soc/sound/soc/sh/rcar/cmd.c:88: error: array subscript is
below array bounds
I've also opened two gcc bugs for warnings that appeared to be issued in error:
https://gcc.gnu.org/bugzilla/show_bug.cgi?idƒ312
https://gcc.gnu.org/bugzilla/show_bug.cgi?id601
I haven't built with gcc-8 in a while, should try that again now that
PR83312 has been
marked 'fixed'.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-01-24 8:43 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-01-22 10:37 [PATCH] [media] s3c-camif: array underflow in __camif_subdev_try_format() Dan Carpenter
2018-01-22 20:50 ` Sylwester Nawrocki
2018-01-24 8:13 ` Dan Carpenter
2018-01-24 8:43 ` Arnd Bergmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox