Hello Dafna, Thanks for your patch!
On 8/29/19 8:00 AM, Dafna Hirschfeld wrote: > Userspace can disable links and create pipelines that > do not start with a source entity. Trying to stream > from such a pipeline should fail with -EPIPE > currently this is not handled and cause kernel crash. > > Signed-off-by: Dafna Hirschfeld <dafna.hirschf...@collabora.com> > --- > Hi, > These are the commands to reproduce the crash: > media-ctl -d0 -l "5:1->21:0[0]" -v > v4l2-ctl -z platform:vimc -d "RGB/YUV Capture" -v width=1920,height=1440 > v4l2-ctl --stream-mmap --stream-count=100 -d /dev/video2 > > drivers/media/platform/vimc/vimc-streamer.c | 39 +++++++++++++++------ > 1 file changed, 28 insertions(+), 11 deletions(-) > > diff --git a/drivers/media/platform/vimc/vimc-streamer.c > b/drivers/media/platform/vimc/vimc-streamer.c > index 048d770e498b..bfc7921c7a8b 100644 > --- a/drivers/media/platform/vimc/vimc-streamer.c > +++ b/drivers/media/platform/vimc/vimc-streamer.c > @@ -13,6 +13,19 @@ > > #include "vimc-streamer.h" > > +/** > + * Check if the entity has only source pads > + */ You should only use /** when you are writing kernel-doc comments. You may either replace /** by /*, or add a function documentation[1]. Thanks, André [1] https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#function-documentation > +static bool vimc_is_source(struct media_entity *ent) > +{ > + int i; > + > + for (i = 0; i < ent->num_pads; i++) > + if (ent->pads[i].flags & MEDIA_PAD_FL_SINK) > + return false; > + return true; > +} > + > /** > * vimc_get_source_entity - get the entity connected with the first sink pad > * > @@ -83,14 +96,12 @@ static int vimc_streamer_pipeline_init(struct vimc_stream > *stream, > struct media_entity *entity; > struct video_device *vdev; > struct v4l2_subdev *sd; > - int ret = 0; > + int ret = -EINVAL; > > stream->pipe_size = 0; > while (stream->pipe_size < VIMC_STREAMER_PIPELINE_MAX_SIZE) { > - if (!ved) { > - vimc_streamer_pipeline_terminate(stream); > - return -EINVAL; > - } > + if (!ved) > + break; > stream->ved_pipeline[stream->pipe_size++] = ved; > > if (is_media_entity_v4l2_subdev(ved->ent)) { > @@ -99,15 +110,22 @@ static int vimc_streamer_pipeline_init(struct > vimc_stream *stream, > if (ret && ret != -ENOIOCTLCMD) { > pr_err("subdev_call error %s\n", > ved->ent->name); > - vimc_streamer_pipeline_terminate(stream); > - return ret; > + break; > } > } > > entity = vimc_get_source_entity(ved->ent); > - /* Check if the end of the pipeline was reached*/ > - if (!entity) > + /* Check if the end of the pipeline was reached */ > + if (!entity) { > + /* the first entity of the pipe should be source only */ > + if (!vimc_is_source(ved->ent)) { > + pr_err("first entity in the pipe '%s' is not a > source\n", > + ved->ent->name); > + ret = -EPIPE; > + break; > + } > return 0; > + } > > /* Get the next device in the pipeline */ > if (is_media_entity_v4l2_subdev(entity)) { > @@ -120,9 +138,8 @@ static int vimc_streamer_pipeline_init(struct vimc_stream > *stream, > ved = video_get_drvdata(vdev); > } > } > - > vimc_streamer_pipeline_terminate(stream); > - return -EINVAL; > + return ret; > } > > /**
