On 14/12/2020 18:16, Michael Tretter wrote: > On Mon, 14 Dec 2020 14:54:47 +0300, Dan Carpenter wrote: >> The "channel" is added to the "dev->channels" but then if >> v4l2_m2m_ctx_init() fails then we free "channel" but it's still on the >> list so it could lead to a use after free. Let's not add it to the >> list until after v4l2_m2m_ctx_init() succeeds. > > Thanks. > > The patch conflicts with the series that moves the driver from staging to > mainline [0]. I'm not sure, which patch should go in first. I'll take care of the conflict. Regards, Hans > > It is also correct to not change the order of list_del and > v4l2_m2m_ctx_release in allegro_release. The list is used to relate messages > from the VCU to their destination channel and this should be possible until > the context has been released and no further messages are expected for that > channel. > > [0] https://lore.kernel.org/linux-media/20201202133040.1954837-1-m.tretter@xxxxxxxxxxxxxx/ > >> >> Fixes: cc62c74749a3 ("media: allegro: add missed checks in allegro_open()") >> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx> > > Reviewed-by: Michael Tretter <m.tretter@xxxxxxxxxxxxxx> > >> --- >> From static analysis. Not tested. >> >> drivers/staging/media/allegro-dvt/allegro-core.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/drivers/staging/media/allegro-dvt/allegro-core.c b/drivers/staging/media/allegro-dvt/allegro-core.c >> index 9f718f43282b..640451134072 100644 >> --- a/drivers/staging/media/allegro-dvt/allegro-core.c >> +++ b/drivers/staging/media/allegro-dvt/allegro-core.c >> @@ -2483,8 +2483,6 @@ static int allegro_open(struct file *file) >> INIT_LIST_HEAD(&channel->buffers_reference); >> INIT_LIST_HEAD(&channel->buffers_intermediate); >> >> - list_add(&channel->list, &dev->channels); >> - >> channel->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->m2m_dev, channel, >> allegro_queue_init); >> >> @@ -2493,6 +2491,7 @@ static int allegro_open(struct file *file) >> goto error; >> } >> >> + list_add(&channel->list, &dev->channels); >> file->private_data = &channel->fh; >> v4l2_fh_add(&channel->fh); >> >> -- >> 2.29.2 >> >>
