On Tue, 2019-11-19 at 18:51 +0100, Dafna Hirschfeld wrote:
> The sd->devnode is release after calling this
s/release/released
> function. Therefore it should be set to NULL so that the
> subdev won't hold a pointer to a released object.
> This fixes a reference after free bug in function
> v4l2_device_unregister_subdev
>
You could mention that this patch fixes a regression, either
in the commit description or in the commit title, or both.
I usually like to put the word "Fix" somewhere in the commit
title, as a hint to people backporting patches.
Something like "Fix subdev release ...".
Also, see below.
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@xxxxxxxxxxxxx>
> ---
> changes since v1: move the line to the funtion v4l2_subdev_release
> from the funtion v4l2_subdev_release
>
> drivers/media/v4l2-core/v4l2-device.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-device.c b/drivers/media/v4l2-core/v4l2-device.c
> index 63d6b147b21e..2b3595671d62 100644
> --- a/drivers/media/v4l2-core/v4l2-device.c
> +++ b/drivers/media/v4l2-core/v4l2-device.c
> @@ -177,6 +177,7 @@ static void v4l2_subdev_release(struct v4l2_subdev *sd)
> {
> struct module *owner = !sd->owner_v4l2_dev ? sd->owner : NULL;
>
> + sd->devnode = NULL;
> if (sd->internal_ops && sd->internal_ops->release)
> sd->internal_ops->release(sd);
> module_put(owner);
Reviewed-by: Ezequiel Garcia <ezequiel@xxxxxxxxxxxxx>
The fix looks good. Interestingly, the devnode = NULL used to be here,
but then got removed when the internal_ops.release was introduced
in commit 0e43734d4c46e156785bb1d2acc5b3c10b7d5dd5.
This means:
Cc: stable@xxxxxxxxxxxxxxx
Fixes: 0e43734d4c46e ("media: v4l2-subdev: add release() internal op")
Thanks,
Ezequiel
