On Fri, Nov 11, 2022 at 09:23:29PM +0800, Yang Yingliang wrote:
> On 2022/11/11 17:19, Laurent Pinchart wrote:
> > On Fri, Nov 11, 2022 at 04:58:09PM +0800, Yang Yingliang wrote:
> >> Afer commit 1fa5ae857bb1 ("driver core: get rid of struct device's
> >> bus_id string array"), the name of device is allocated dynamically,
> >> so call kfree_const() to freed it in error case.
> >>
> >> Fixes: 1fa5ae857bb1 ("driver core: get rid of struct device's bus_id string array")
> >> Signed-off-by: Yang Yingliang <yangyingliang@xxxxxxxxxx>
> >> ---
> >> drivers/media/v4l2-core/v4l2-dev.c | 3 +++
> >> 1 file changed, 3 insertions(+)
> >>
> >> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
> >> index 397d553177fa..1fce1c29c60f 100644
> >> --- a/drivers/media/v4l2-core/v4l2-dev.c
> >> +++ b/drivers/media/v4l2-core/v4l2-dev.c
> >> @@ -1036,6 +1036,9 @@ int __video_register_device(struct video_device *vdev,
> >> ret = device_register(&vdev->dev);
> >> if (ret < 0) {
> >> pr_err("%s: device_register failed\n", __func__);
> >> + kfree_const(vdev->dev.kobj.name);
> >> + /* set it to null to avoid callers using a bad pointer */
> >> + vdev->dev.kobj.name = NULL;
> >
> > This doesn't seem right to me. We shouldn't be poking into the kobj.
> > This should be handled in the driver core, or if that can't be done for
> > a reason specific to how the device is used in the V4L2 core, we need a
> > helper function from the driver core to perform the cleanup.
>
> In technical, we should call put_device() here, but the release()
> function has not been set in this case, and v4l2_device_release() will
> release something that not need be, so we can not handle it in the
> driver core well, I think free the name here directly is the best way
> to fix it.
The documentation of device_register() states
* NOTE: _Never_ directly free @dev after calling this function, even
* if it returned an error! Always use put_device() to give up the
* reference initialized in this function instead.
We thus need to fix this in __video_register_device(), not to pile
another workaround :-)
> >> goto cleanup;
> >> }
> >> /* Register the release callback that will be called when the last
--
Regards,
Laurent Pinchart
