All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] media: camss: Fix signedness bug in video_enum_fmt()
@ 2020-12-09  6:50 ` Dan Carpenter
  0 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2020-12-09  6:50 UTC (permalink / raw)
  To: Robert Foss, Andrey Konovalov
  Cc: Todor Tomov, Andy Gross, Bjorn Andersson, Mauro Carvalho Chehab,
	Hans Verkuil, linux-media, kernel-janitors

This test has a problem because we want to know if "k" is -1 or a
positive value less than "f->index".  But the "f->index" variable is a
u32 so if "k == -1" then -1 gets type promoted to UINT_MAX which is
larger than "f->index".  I've added an explicit test to check for -1.

Fixes: a3d412d4b9f3 ("media: Revert "media: camss: Make use of V4L2_CAP_IO_MC"")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
I could equally well have casted "k < (int)f->index" but I feel like
this is more explicit and readable.

 drivers/media/platform/qcom/camss/camss-video.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/qcom/camss/camss-video.c b/drivers/media/platform/qcom/camss/camss-video.c
index bd9334af1c73..2fa3214775d5 100644
--- a/drivers/media/platform/qcom/camss/camss-video.c
+++ b/drivers/media/platform/qcom/camss/camss-video.c
@@ -579,7 +579,7 @@ static int video_enum_fmt(struct file *file, void *fh, struct v4l2_fmtdesc *f)
 			break;
 	}
 
-	if (k < f->index)
+	if (k == -1 || k < f->index)
 		/*
 		 * All the unique pixel formats matching the arguments
 		 * have been enumerated (k >= 0 and f->index > 0), or
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH 1/2] media: camss: Fix signedness bug in video_enum_fmt()
@ 2020-12-09  6:50 ` Dan Carpenter
  0 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2020-12-09  6:50 UTC (permalink / raw)
  To: Robert Foss, Andrey Konovalov
  Cc: Todor Tomov, Andy Gross, Bjorn Andersson, Mauro Carvalho Chehab,
	Hans Verkuil, linux-media, kernel-janitors

This test has a problem because we want to know if "k" is -1 or a
positive value less than "f->index".  But the "f->index" variable is a
u32 so if "k = -1" then -1 gets type promoted to UINT_MAX which is
larger than "f->index".  I've added an explicit test to check for -1.

Fixes: a3d412d4b9f3 ("media: Revert "media: camss: Make use of V4L2_CAP_IO_MC"")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
I could equally well have casted "k < (int)f->index" but I feel like
this is more explicit and readable.

 drivers/media/platform/qcom/camss/camss-video.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/qcom/camss/camss-video.c b/drivers/media/platform/qcom/camss/camss-video.c
index bd9334af1c73..2fa3214775d5 100644
--- a/drivers/media/platform/qcom/camss/camss-video.c
+++ b/drivers/media/platform/qcom/camss/camss-video.c
@@ -579,7 +579,7 @@ static int video_enum_fmt(struct file *file, void *fh, struct v4l2_fmtdesc *f)
 			break;
 	}
 
-	if (k < f->index)
+	if (k = -1 || k < f->index)
 		/*
 		 * All the unique pixel formats matching the arguments
 		 * have been enumerated (k >= 0 and f->index > 0), or
-- 
2.29.2

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH 2/2] media: camss: missing error code in msm_video_register()
  2020-12-09  6:50 ` Dan Carpenter
@ 2020-12-09  6:51   ` Dan Carpenter
  -1 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2020-12-09  6:51 UTC (permalink / raw)
  To: Robert Foss, Todor Tomov
  Cc: Andy Gross, Bjorn Andersson, Mauro Carvalho Chehab, Hans Verkuil,
	linux-media, kernel-janitors

This error path returns success but it should return -EINVAL.

Fixes: cba3819d1e93 ("media: camss: Format configuration per hardware version")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 drivers/media/platform/qcom/camss/camss-video.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/platform/qcom/camss/camss-video.c b/drivers/media/platform/qcom/camss/camss-video.c
index 2fa3214775d5..97cea7c4d769 100644
--- a/drivers/media/platform/qcom/camss/camss-video.c
+++ b/drivers/media/platform/qcom/camss/camss-video.c
@@ -961,6 +961,7 @@ int msm_video_register(struct camss_video *video, struct v4l2_device *v4l2_dev,
 			video->nformats = ARRAY_SIZE(formats_rdi_8x96);
 		}
 	} else {
+		ret = -EINVAL;
 		goto error_video_register;
 	}
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH 2/2] media: camss: missing error code in msm_video_register()
@ 2020-12-09  6:51   ` Dan Carpenter
  0 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2020-12-09  6:51 UTC (permalink / raw)
  To: Robert Foss, Todor Tomov
  Cc: Andy Gross, Bjorn Andersson, Mauro Carvalho Chehab, Hans Verkuil,
	linux-media, kernel-janitors

This error path returns success but it should return -EINVAL.

Fixes: cba3819d1e93 ("media: camss: Format configuration per hardware version")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 drivers/media/platform/qcom/camss/camss-video.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/platform/qcom/camss/camss-video.c b/drivers/media/platform/qcom/camss/camss-video.c
index 2fa3214775d5..97cea7c4d769 100644
--- a/drivers/media/platform/qcom/camss/camss-video.c
+++ b/drivers/media/platform/qcom/camss/camss-video.c
@@ -961,6 +961,7 @@ int msm_video_register(struct camss_video *video, struct v4l2_device *v4l2_dev,
 			video->nformats = ARRAY_SIZE(formats_rdi_8x96);
 		}
 	} else {
+		ret = -EINVAL;
 		goto error_video_register;
 	}
 
-- 
2.29.2

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] media: camss: Fix signedness bug in video_enum_fmt()
  2020-12-09  6:50 ` Dan Carpenter
@ 2020-12-09 10:55   ` Robert Foss
  -1 siblings, 0 replies; 10+ messages in thread
From: Robert Foss @ 2020-12-09 10:55 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Andrey Konovalov, Todor Tomov, Andy Gross, Bjorn Andersson,
	Mauro Carvalho Chehab, Hans Verkuil, linux-media,
	kernel-janitors

Hey Dan,

Thank you for submitting this.

Reviewed-by: Robert Foss <robert.foss@linaro.org>

On Wed, 9 Dec 2020 at 07:50, Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> This test has a problem because we want to know if "k" is -1 or a
> positive value less than "f->index".  But the "f->index" variable is a
> u32 so if "k == -1" then -1 gets type promoted to UINT_MAX which is
> larger than "f->index".  I've added an explicit test to check for -1.
>
> Fixes: a3d412d4b9f3 ("media: Revert "media: camss: Make use of V4L2_CAP_IO_MC"")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> I could equally well have casted "k < (int)f->index" but I feel like
> this is more explicit and readable.
>
>  drivers/media/platform/qcom/camss/camss-video.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-video.c b/drivers/media/platform/qcom/camss/camss-video.c
> index bd9334af1c73..2fa3214775d5 100644
> --- a/drivers/media/platform/qcom/camss/camss-video.c
> +++ b/drivers/media/platform/qcom/camss/camss-video.c
> @@ -579,7 +579,7 @@ static int video_enum_fmt(struct file *file, void *fh, struct v4l2_fmtdesc *f)
>                         break;
>         }
>
> -       if (k < f->index)
> +       if (k == -1 || k < f->index)
>                 /*
>                  * All the unique pixel formats matching the arguments
>                  * have been enumerated (k >= 0 and f->index > 0), or
> --
> 2.29.2
>

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] media: camss: Fix signedness bug in video_enum_fmt()
@ 2020-12-09 10:55   ` Robert Foss
  0 siblings, 0 replies; 10+ messages in thread
From: Robert Foss @ 2020-12-09 10:55 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Andrey Konovalov, Todor Tomov, Andy Gross, Bjorn Andersson,
	Mauro Carvalho Chehab, Hans Verkuil, linux-media,
	kernel-janitors

Hey Dan,

Thank you for submitting this.

Reviewed-by: Robert Foss <robert.foss@linaro.org>

On Wed, 9 Dec 2020 at 07:50, Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> This test has a problem because we want to know if "k" is -1 or a
> positive value less than "f->index".  But the "f->index" variable is a
> u32 so if "k = -1" then -1 gets type promoted to UINT_MAX which is
> larger than "f->index".  I've added an explicit test to check for -1.
>
> Fixes: a3d412d4b9f3 ("media: Revert "media: camss: Make use of V4L2_CAP_IO_MC"")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> I could equally well have casted "k < (int)f->index" but I feel like
> this is more explicit and readable.
>
>  drivers/media/platform/qcom/camss/camss-video.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-video.c b/drivers/media/platform/qcom/camss/camss-video.c
> index bd9334af1c73..2fa3214775d5 100644
> --- a/drivers/media/platform/qcom/camss/camss-video.c
> +++ b/drivers/media/platform/qcom/camss/camss-video.c
> @@ -579,7 +579,7 @@ static int video_enum_fmt(struct file *file, void *fh, struct v4l2_fmtdesc *f)
>                         break;
>         }
>
> -       if (k < f->index)
> +       if (k = -1 || k < f->index)
>                 /*
>                  * All the unique pixel formats matching the arguments
>                  * have been enumerated (k >= 0 and f->index > 0), or
> --
> 2.29.2
>

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] media: camss: missing error code in msm_video_register()
  2020-12-09  6:51   ` Dan Carpenter
@ 2020-12-09 10:56     ` Robert Foss
  -1 siblings, 0 replies; 10+ messages in thread
From: Robert Foss @ 2020-12-09 10:56 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Todor Tomov, Andy Gross, Bjorn Andersson, Mauro Carvalho Chehab,
	Hans Verkuil, linux-media, kernel-janitors

Thanks for submitting this Dan.

Reviewed-by: Robert Foss <robert.foss@linaro.org>

On Wed, 9 Dec 2020 at 07:51, Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> This error path returns success but it should return -EINVAL.
>
> Fixes: cba3819d1e93 ("media: camss: Format configuration per hardware version")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
>  drivers/media/platform/qcom/camss/camss-video.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-video.c b/drivers/media/platform/qcom/camss/camss-video.c
> index 2fa3214775d5..97cea7c4d769 100644
> --- a/drivers/media/platform/qcom/camss/camss-video.c
> +++ b/drivers/media/platform/qcom/camss/camss-video.c
> @@ -961,6 +961,7 @@ int msm_video_register(struct camss_video *video, struct v4l2_device *v4l2_dev,
>                         video->nformats = ARRAY_SIZE(formats_rdi_8x96);
>                 }
>         } else {
> +               ret = -EINVAL;
>                 goto error_video_register;
>         }
>
> --
> 2.29.2
>

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] media: camss: missing error code in msm_video_register()
@ 2020-12-09 10:56     ` Robert Foss
  0 siblings, 0 replies; 10+ messages in thread
From: Robert Foss @ 2020-12-09 10:56 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Todor Tomov, Andy Gross, Bjorn Andersson, Mauro Carvalho Chehab,
	Hans Verkuil, linux-media, kernel-janitors

Thanks for submitting this Dan.

Reviewed-by: Robert Foss <robert.foss@linaro.org>

On Wed, 9 Dec 2020 at 07:51, Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> This error path returns success but it should return -EINVAL.
>
> Fixes: cba3819d1e93 ("media: camss: Format configuration per hardware version")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
>  drivers/media/platform/qcom/camss/camss-video.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-video.c b/drivers/media/platform/qcom/camss/camss-video.c
> index 2fa3214775d5..97cea7c4d769 100644
> --- a/drivers/media/platform/qcom/camss/camss-video.c
> +++ b/drivers/media/platform/qcom/camss/camss-video.c
> @@ -961,6 +961,7 @@ int msm_video_register(struct camss_video *video, struct v4l2_device *v4l2_dev,
>                         video->nformats = ARRAY_SIZE(formats_rdi_8x96);
>                 }
>         } else {
> +               ret = -EINVAL;
>                 goto error_video_register;
>         }
>
> --
> 2.29.2
>

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] media: camss: Fix signedness bug in video_enum_fmt()
  2020-12-09 10:55   ` Robert Foss
@ 2020-12-09 22:04     ` Andrey Konovalov
  -1 siblings, 0 replies; 10+ messages in thread
From: Andrey Konovalov @ 2020-12-09 22:04 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Robert Foss, Todor Tomov, Andy Gross, Bjorn Andersson,
	Mauro Carvalho Chehab, Hans Verkuil, linux-media,
	kernel-janitors

Hi Dan,

Good catch!

Reviewed-by: Andrey Konovalov <andrey.konovalov@linaro.org>

Thanks,
Andrey

On 09.12.2020 13:55, Robert Foss wrote:
> Hey Dan,
> 
> Thank you for submitting this.
> 
> Reviewed-by: Robert Foss <robert.foss@linaro.org>
> 
> On Wed, 9 Dec 2020 at 07:50, Dan Carpenter <dan.carpenter@oracle.com> wrote:
>>
>> This test has a problem because we want to know if "k" is -1 or a
>> positive value less than "f->index".  But the "f->index" variable is a
>> u32 so if "k == -1" then -1 gets type promoted to UINT_MAX which is
>> larger than "f->index".  I've added an explicit test to check for -1.
>>
>> Fixes: a3d412d4b9f3 ("media: Revert "media: camss: Make use of V4L2_CAP_IO_MC"")
>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>> ---
>> I could equally well have casted "k < (int)f->index" but I feel like
>> this is more explicit and readable.
>>
>>   drivers/media/platform/qcom/camss/camss-video.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/platform/qcom/camss/camss-video.c b/drivers/media/platform/qcom/camss/camss-video.c
>> index bd9334af1c73..2fa3214775d5 100644
>> --- a/drivers/media/platform/qcom/camss/camss-video.c
>> +++ b/drivers/media/platform/qcom/camss/camss-video.c
>> @@ -579,7 +579,7 @@ static int video_enum_fmt(struct file *file, void *fh, struct v4l2_fmtdesc *f)
>>                          break;
>>          }
>>
>> -       if (k < f->index)
>> +       if (k == -1 || k < f->index)
>>                  /*
>>                   * All the unique pixel formats matching the arguments
>>                   * have been enumerated (k >= 0 and f->index > 0), or
>> --
>> 2.29.2
>>

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] media: camss: Fix signedness bug in video_enum_fmt()
@ 2020-12-09 22:04     ` Andrey Konovalov
  0 siblings, 0 replies; 10+ messages in thread
From: Andrey Konovalov @ 2020-12-09 22:04 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Robert Foss, Todor Tomov, Andy Gross, Bjorn Andersson,
	Mauro Carvalho Chehab, Hans Verkuil, linux-media,
	kernel-janitors

Hi Dan,

Good catch!

Reviewed-by: Andrey Konovalov <andrey.konovalov@linaro.org>

Thanks,
Andrey

On 09.12.2020 13:55, Robert Foss wrote:
> Hey Dan,
> 
> Thank you for submitting this.
> 
> Reviewed-by: Robert Foss <robert.foss@linaro.org>
> 
> On Wed, 9 Dec 2020 at 07:50, Dan Carpenter <dan.carpenter@oracle.com> wrote:
>>
>> This test has a problem because we want to know if "k" is -1 or a
>> positive value less than "f->index".  But the "f->index" variable is a
>> u32 so if "k = -1" then -1 gets type promoted to UINT_MAX which is
>> larger than "f->index".  I've added an explicit test to check for -1.
>>
>> Fixes: a3d412d4b9f3 ("media: Revert "media: camss: Make use of V4L2_CAP_IO_MC"")
>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>> ---
>> I could equally well have casted "k < (int)f->index" but I feel like
>> this is more explicit and readable.
>>
>>   drivers/media/platform/qcom/camss/camss-video.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/platform/qcom/camss/camss-video.c b/drivers/media/platform/qcom/camss/camss-video.c
>> index bd9334af1c73..2fa3214775d5 100644
>> --- a/drivers/media/platform/qcom/camss/camss-video.c
>> +++ b/drivers/media/platform/qcom/camss/camss-video.c
>> @@ -579,7 +579,7 @@ static int video_enum_fmt(struct file *file, void *fh, struct v4l2_fmtdesc *f)
>>                          break;
>>          }
>>
>> -       if (k < f->index)
>> +       if (k = -1 || k < f->index)
>>                  /*
>>                   * All the unique pixel formats matching the arguments
>>                   * have been enumerated (k >= 0 and f->index > 0), or
>> --
>> 2.29.2
>>

^ permalink raw reply	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-12-09 22:05 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-09  6:50 [PATCH 1/2] media: camss: Fix signedness bug in video_enum_fmt() Dan Carpenter
2020-12-09  6:50 ` Dan Carpenter
2020-12-09  6:51 ` [PATCH 2/2] media: camss: missing error code in msm_video_register() Dan Carpenter
2020-12-09  6:51   ` Dan Carpenter
2020-12-09 10:56   ` Robert Foss
2020-12-09 10:56     ` Robert Foss
2020-12-09 10:55 ` [PATCH 1/2] media: camss: Fix signedness bug in video_enum_fmt() Robert Foss
2020-12-09 10:55   ` Robert Foss
2020-12-09 22:04   ` Andrey Konovalov
2020-12-09 22:04     ` Andrey Konovalov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.