lkml.org 
[lkml]   [2022]   [Dec]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] media: staging: media: omap4iss: Fix null dereference for iss
Hi Tanmay,

Thank you for the patch.

On Wed, Dec 28, 2022 at 09:58:31PM +0100, Tanmay Bhushan wrote:
> From 7aa39c0d02bddf9cfa14762f115303b79bfa0ae3 Mon Sep 17 00:00:00 2001
> From: Tanmay Bhushan <007047221b@gmail.com>
> Date: Wed, 28 Dec 2022 21:01:16 +0100
> Subject: [PATCH] media: staging: media: omap4iss: Fix null dereference
> for iss
>
> media_pad_remote_pad_first returns NULL in some cases but while using
> the return value was used without NULL check which will lead to panic
> in case of NULL return. iss_pipeline_is_last returns value check so
> have returned 0 in case of NULL and csi2_configure is not documented
> for such cases so returned EINVAL for it. Code is not tested
> as it is only for NULL dereference verification.
>
> Signed-off-by: Tanmay Bhushan <007047221b@gmail.com>
> ---
> drivers/staging/media/omap4iss/iss.c | 6 +++++-
> drivers/staging/media/omap4iss/iss_csi2.c | 4 ++++
> 2 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/omap4iss/iss.c
> b/drivers/staging/media/omap4iss/iss.c
> index fa2a36d829d3..3f01eeff40e7 100644
> --- a/drivers/staging/media/omap4iss/iss.c
> +++ b/drivers/staging/media/omap4iss/iss.c
> @@ -552,7 +552,11 @@ static int iss_pipeline_is_last(struct
> media_entity *me)

Your mail client wrapped lines, which prevents the patch from being
applied with git-am. I recommend using git-send-email to send patches.
https://git-send-email.io/ provides clear and detailed instructions on
how to set it up (especially when using gmail).

> if (!pipe || pipe->stream_state ==
> ISS_PIPELINE_STREAM_STOPPED)
> return 0;
> pad = media_pad_remote_pad_first(&pipe->output->pad);
> - return pad->entity == me;

Have you seen this actually crashing, or are you only speculating ? The
video node at the output of the pipeline should always be connected, so
I don't think media_pad_remote_pad_first() can ever return NULL here.

> +
> + if (pad)
> + return pad->entity == me;
> +
> + return 0;
> }
>
> static int iss_reset(struct iss_device *iss)
> diff --git a/drivers/staging/media/omap4iss/iss_csi2.c
> b/drivers/staging/media/omap4iss/iss_csi2.c
> index 04ce0e7eb557..ab2c2ad64464 100644
> --- a/drivers/staging/media/omap4iss/iss_csi2.c
> +++ b/drivers/staging/media/omap4iss/iss_csi2.c
> @@ -539,6 +539,10 @@ static int csi2_configure(struct iss_csi2_device
> *csi2)
> return -EBUSY;
>
> pad = media_pad_remote_pad_first(&csi2->pads[CSI2_PAD_SINK]);
> +
> + if (!pad)
> + return -EINVAL;

Same here, what makes you think this is possible ?

> +
> sensor = media_entity_to_v4l2_subdev(pad->entity);
> pdata = sensor->host_priv;
>
>

--
Regards,

Laurent Pinchart

\
 
 \ /
  Last update: 2023-03-26 23:22    [W:0.038 / U:1.520 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site