Re: [PATCH] media: allegro: Fix use after free on error
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.tret...@pengutronix.de/
>
>>
>> Fixes: cc62c74749a3 ("media: allegro: add missed checks in allegro_open()")
>> Signed-off-by: Dan Carpenter
>
> Reviewed-by: Michael Tretter
>
>> ---
>> 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
>>
>>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v6 02/11] firmware: raspberrypi: Introduce devm_rpi_firmware_get()
On Fri, Dec 11, 2020 at 5:48 PM Nicolas Saenz Julienne
wrote:
>
> It'll simplify the firmware handling for most consumers.
>
> Suggested-by: Bartosz Golaszewski
> Signed-off-by: Nicolas Saenz Julienne
> Reviewed-by: Florian Fainelli
> ---
>
> Changes since v4:
> - Rearrange function calls for clarity, same functionality
>
> Changes since v2:
> - Create devm_rpi_firmware_get()
>
> drivers/firmware/raspberrypi.c | 29 ++
> include/soc/bcm2835/raspberrypi-firmware.h | 8 ++
> 2 files changed, 37 insertions(+)
>
> diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c
> index b65e4c495772..250e01680742 100644
> --- a/drivers/firmware/raspberrypi.c
> +++ b/drivers/firmware/raspberrypi.c
> @@ -243,6 +243,13 @@ void rpi_firmware_put(struct rpi_firmware *fw)
> }
> EXPORT_SYMBOL_GPL(rpi_firmware_put);
>
> +static void devm_rpi_firmware_put(void *data)
> +{
> + struct rpi_firmware *fw = data;
> +
> + rpi_firmware_put(fw);
> +}
> +
> static int rpi_firmware_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> @@ -331,6 +338,28 @@ struct rpi_firmware *rpi_firmware_get(struct device_node
> *firmware_node)
> }
> EXPORT_SYMBOL_GPL(rpi_firmware_get);
>
> +/**
> + * devm_rpi_firmware_get - Get pointer to rpi_firmware structure.
> + * @firmware_node:Pointer to the firmware Device Tree node.
> + *
> + * Returns NULL is the firmware device is not ready.
> + */
> +struct rpi_firmware *devm_rpi_firmware_get(struct device *dev,
> + struct device_node *firmware_node)
> +{
> + struct rpi_firmware *fw;
> +
> + fw = rpi_firmware_get(firmware_node);
> + if (!fw)
> + return NULL;
> +
> + if (devm_add_action_or_reset(dev, devm_rpi_firmware_put, fw))
> + return NULL;
> +
> + return fw;
> +}
> +EXPORT_SYMBOL_GPL(devm_rpi_firmware_get);
> +
> static const struct of_device_id rpi_firmware_of_match[] = {
> { .compatible = "raspberrypi,bcm2835-firmware", },
> {},
> diff --git a/include/soc/bcm2835/raspberrypi-firmware.h
> b/include/soc/bcm2835/raspberrypi-firmware.h
> index fdfef7fe40df..73ad784fca96 100644
> --- a/include/soc/bcm2835/raspberrypi-firmware.h
> +++ b/include/soc/bcm2835/raspberrypi-firmware.h
> @@ -142,6 +142,8 @@ int rpi_firmware_property_list(struct rpi_firmware *fw,
>void *data, size_t tag_size);
> void rpi_firmware_put(struct rpi_firmware *fw);
> struct rpi_firmware *rpi_firmware_get(struct device_node *firmware_node);
> +struct rpi_firmware *devm_rpi_firmware_get(struct device *dev,
> + struct device_node *firmware_node);
> #else
> static inline int rpi_firmware_property(struct rpi_firmware *fw, u32 tag,
> void *data, size_t len)
> @@ -160,6 +162,12 @@ static inline struct rpi_firmware
> *rpi_firmware_get(struct device_node *firmware
> {
> return NULL;
> }
> +
> +static inline struct rpi_firmware *devm_rpi_firmware_get(struct device *dev,
> + struct device_node *firmware_node)
> +{
> + return NULL;
> +}
> #endif
>
> #endif /* __SOC_RASPBERRY_FIRMWARE_H__ */
> --
> 2.29.2
>
Reviewed-by: Bartosz Golaszewski
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v6 02/11] firmware: raspberrypi: Introduce devm_rpi_firmware_get()
On Wed, 2020-12-16 at 11:35 +0100, Bartosz Golaszewski wrote:
> On Fri, Dec 11, 2020 at 5:48 PM Nicolas Saenz Julienne
> wrote:
> >
> > It'll simplify the firmware handling for most consumers.
> >
> > Suggested-by: Bartosz Golaszewski
> > Signed-off-by: Nicolas Saenz Julienne
> > Reviewed-by: Florian Fainelli
> > ---
> >
> > Changes since v4:
> > - Rearrange function calls for clarity, same functionality
> >
> > Changes since v2:
> > - Create devm_rpi_firmware_get()
> >
> > drivers/firmware/raspberrypi.c | 29 ++
> > include/soc/bcm2835/raspberrypi-firmware.h | 8 ++
> > 2 files changed, 37 insertions(+)
> >
> > diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c
> > index b65e4c495772..250e01680742 100644
> > --- a/drivers/firmware/raspberrypi.c
> > +++ b/drivers/firmware/raspberrypi.c
> > @@ -243,6 +243,13 @@ void rpi_firmware_put(struct rpi_firmware *fw)
> > }
> > EXPORT_SYMBOL_GPL(rpi_firmware_put);
> >
> > +static void devm_rpi_firmware_put(void *data)
> > +{
> > + struct rpi_firmware *fw = data;
> > +
> > + rpi_firmware_put(fw);
> > +}
> > +
> > static int rpi_firmware_probe(struct platform_device *pdev)
> > {
> > struct device *dev = &pdev->dev;
> > @@ -331,6 +338,28 @@ struct rpi_firmware *rpi_firmware_get(struct
> > device_node *firmware_node)
> > }
> > EXPORT_SYMBOL_GPL(rpi_firmware_get);
> >
> > +/**
> > + * devm_rpi_firmware_get - Get pointer to rpi_firmware structure.
> > + * @firmware_node:Pointer to the firmware Device Tree node.
> > + *
> > + * Returns NULL is the firmware device is not ready.
> > + */
> > +struct rpi_firmware *devm_rpi_firmware_get(struct device *dev,
> > + struct device_node
> > *firmware_node)
> > +{
> > + struct rpi_firmware *fw;
> > +
> > + fw = rpi_firmware_get(firmware_node);
> > + if (!fw)
> > + return NULL;
> > +
> > + if (devm_add_action_or_reset(dev, devm_rpi_firmware_put, fw))
> > + return NULL;
> > +
> > + return fw;
> > +}
> > +EXPORT_SYMBOL_GPL(devm_rpi_firmware_get);
> > +
> > static const struct of_device_id rpi_firmware_of_match[] = {
> > { .compatible = "raspberrypi,bcm2835-firmware", },
> > {},
> > diff --git a/include/soc/bcm2835/raspberrypi-firmware.h
> > b/include/soc/bcm2835/raspberrypi-firmware.h
> > index fdfef7fe40df..73ad784fca96 100644
> > --- a/include/soc/bcm2835/raspberrypi-firmware.h
> > +++ b/include/soc/bcm2835/raspberrypi-firmware.h
> > @@ -142,6 +142,8 @@ int rpi_firmware_property_list(struct rpi_firmware *fw,
> > void *data, size_t tag_size);
> > void rpi_firmware_put(struct rpi_firmware *fw);
> > struct rpi_firmware *rpi_firmware_get(struct device_node *firmware_node);
> > +struct rpi_firmware *devm_rpi_firmware_get(struct device *dev,
> > + struct device_node
> > *firmware_node);
> > #else
> > static inline int rpi_firmware_property(struct rpi_firmware *fw, u32 tag,
> > void *data, size_t len)
> > @@ -160,6 +162,12 @@ static inline struct rpi_firmware
> > *rpi_firmware_get(struct device_node *firmware
> > {
> > return NULL;
> > }
> > +
> > +static inline struct rpi_firmware *devm_rpi_firmware_get(struct device
> > *dev,
> > + struct device_node *firmware_node)
> > +{
> > + return NULL;
> > +}
> > #endif
> >
> > #endif /* __SOC_RASPBERRY_FIRMWARE_H__ */
> > --
> > 2.29.2
> >
>
> Reviewed-by: Bartosz Golaszewski
Thanks for the reviews!
signature.asc
Description: This is a digitally signed message part
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v6 01/11] firmware: raspberrypi: Keep count of all consumers
On Fri, Dec 11, 2020 at 5:48 PM Nicolas Saenz Julienne wrote: > > When unbinding the firmware device we need to make sure it has no > consumers left. Otherwise we'd leave them with a firmware handle > pointing at freed memory. > > Keep a reference count of all consumers and introduce rpi_firmware_put() > which will permit automatically decrease the reference count upon > unbinding consumer drivers. > > Suggested-by: Uwe Kleine-König > Signed-off-by: Nicolas Saenz Julienne > Reviewed-by: Florian Fainelli > > --- Reviewed-by: Bartosz Golaszewski ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
