On 11/15/19 11:12 PM, Ezequiel Garcia wrote:
Hi Dafna, Thanks for the patch! On Fri, 15 Nov 2019 at 13:13, Dafna Hirschfeld <dafna.hirschfeld@xxxxxxxxxxxxx> wrote:In case v4l2_device_register_subdev_nodes fails, sd->devnode is unregistered and then released. Therefore the field is 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 Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@xxxxxxxxxxxxx> --- 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..468705c85e97 100644 --- a/drivers/media/v4l2-core/v4l2-device.c +++ b/drivers/media/v4l2-core/v4l2-device.c @@ -250,6 +250,7 @@ int v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev) if (!sd->devnode) break; video_unregister_device(sd->devnode); + sd->devnode = NULL; }You also need to clear devnode in v4l2_device_unregister_subdev, as we discussed in IRC. It would mean fixing a different issue that is triggered upon driver removal/unbinding,
I think that the right place to set devnode to null would be in the beginning of the function v4l2_device_release_subdev_node where it is freed. This ensures that the devnode is set to NULL always and only when it is freed. What do you think? Thanks, Dafna
but it seems fine to address both issues as one: don't leave devnode incorrectly defined. Can you send a new version (and amend the commit description)? Thanks, Ezequiel
