2019-06-26 18:53:21

by Steve Longerbeam

[permalink] [raw]
Subject: [PATCH] media: staging/imx: Fix NULL deref in find_pipeline_entity()

Fix a cut&paste error in find_pipeline_entity(). The start entity must be
passed to media_entity_to_video_device() in find_pipeline_entity(), not
pad->entity. The pad is only put to use later, after determining the start
entity is not the entity being searched for.

Fixes: 3ef46bc97ca2 ("media: staging/imx: Improve pipeline searching")

Reported-by: Colin Ian King <[email protected]>
Signed-off-by: Steve Longerbeam <[email protected]>
---
drivers/staging/media/imx/imx-media-utils.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
index b5b8a3b7730a..6fb88c22ee27 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -842,7 +842,7 @@ find_pipeline_entity(struct media_entity *start, u32 grp_id,
if (sd->grp_id & grp_id)
return &sd->entity;
} else if (buftype && is_media_entity_v4l2_video_device(start)) {
- vfd = media_entity_to_video_device(pad->entity);
+ vfd = media_entity_to_video_device(start);
if (buftype == vfd->queue->type)
return &vfd->entity;
}
--
2.17.1


2019-06-27 06:35:11

by Philipp Zabel

[permalink] [raw]
Subject: Re: [PATCH] media: staging/imx: Fix NULL deref in find_pipeline_entity()

On Wed, 2019-06-26 at 11:52 -0700, Steve Longerbeam wrote:
> Fix a cut&paste error in find_pipeline_entity(). The start entity must be
> passed to media_entity_to_video_device() in find_pipeline_entity(), not
> pad->entity. The pad is only put to use later, after determining the start
> entity is not the entity being searched for.
>
> Fixes: 3ef46bc97ca2 ("media: staging/imx: Improve pipeline searching")
>
> Reported-by: Colin Ian King <[email protected]>
> Signed-off-by: Steve Longerbeam <[email protected]>
> ---
> drivers/staging/media/imx/imx-media-utils.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
> index b5b8a3b7730a..6fb88c22ee27 100644
> --- a/drivers/staging/media/imx/imx-media-utils.c
> +++ b/drivers/staging/media/imx/imx-media-utils.c
> @@ -842,7 +842,7 @@ find_pipeline_entity(struct media_entity *start, u32 grp_id,
> if (sd->grp_id & grp_id)
> return &sd->entity;
> } else if (buftype && is_media_entity_v4l2_video_device(start)) {
> - vfd = media_entity_to_video_device(pad->entity);
> + vfd = media_entity_to_video_device(start);
> if (buftype == vfd->queue->type)
> return &vfd->entity;
> }

Reviewed-by: Philipp Zabel <[email protected]>

regards
Philipp

2019-06-27 07:07:52

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH] media: staging/imx: Fix NULL deref in find_pipeline_entity()

On Wed, Jun 26, 2019 at 11:52:25AM -0700, Steve Longerbeam wrote:
> Fix a cut&paste error in find_pipeline_entity(). The start entity must be
> passed to media_entity_to_video_device() in find_pipeline_entity(), not
> pad->entity. The pad is only put to use later, after determining the start
> entity is not the entity being searched for.
>
> Fixes: 3ef46bc97ca2 ("media: staging/imx: Improve pipeline searching")
>
> Reported-by: Colin Ian King <[email protected]>
> Signed-off-by: Steve Longerbeam <[email protected]>
> ---
> drivers/staging/media/imx/imx-media-utils.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
> index b5b8a3b7730a..6fb88c22ee27 100644
> --- a/drivers/staging/media/imx/imx-media-utils.c
> +++ b/drivers/staging/media/imx/imx-media-utils.c
> @@ -842,7 +842,7 @@ find_pipeline_entity(struct media_entity *start, u32 grp_id,
> if (sd->grp_id & grp_id)
> return &sd->entity;
> } else if (buftype && is_media_entity_v4l2_video_device(start)) {
> - vfd = media_entity_to_video_device(pad->entity);
> + vfd = media_entity_to_video_device(start);

Can we also remove the "pad = NULL" assignment at the start of the
function? Otherwise static checkers and new versions of GCC will warn
that the assignment isn't used. Plus removing the initialization will
prevent bugs like this in the future.

regards,
dan carpenter