I belive this is also addressed by this patch that is under review: https://patchwork.linuxtv.org/project/linux-media/patch/20211008120914.69175-1-ribalda@xxxxxxxxxxxx/ On Thu, 11 Nov 2021 at 11:33, Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote: > > On Thu, Nov 11, 2021 at 09:06:11AM +0100, Christophe JAILLET wrote: > > If 'map->name' can't be allocated, 'map' must be released before returning. > > > > Fixes: 70fa906d6fce ("media: uvcvideo: Use control names from framework") > > Signed-off-by: Christophe JAILLET <christophe.jaillet@xxxxxxxxxx> > > --- > > drivers/media/usb/uvc/uvc_v4l2.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c > > index f4e4aff8ddf7..5aa76a9a6080 100644 > > --- a/drivers/media/usb/uvc/uvc_v4l2.c > > +++ b/drivers/media/usb/uvc/uvc_v4l2.c > > @@ -44,8 +44,10 @@ static int uvc_ioctl_ctrl_map(struct uvc_video_chain *chain, > > if (v4l2_ctrl_get_name(map->id) == NULL) { > > map->name = kmemdup(xmap->name, sizeof(xmap->name), > > GFP_KERNEL); > > - if (!map->name) > > + if (!map->name) { > > + kfree(map); > > return -ENOMEM; > > + } > > Your patch is fine but there is a second issue. The error handling > should free "map->name" as well. The problem is that this function > frees everything on the success path at all, but freeing map->name on > the success path will lead to a crash so you have to do something > weird like: > > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c > index f4e4aff8ddf7..953a5cbf7945 100644 > --- a/drivers/media/usb/uvc/uvc_v4l2.c > +++ b/drivers/media/usb/uvc/uvc_v4l2.c > @@ -90,6 +90,9 @@ static int uvc_ioctl_ctrl_map(struct uvc_video_chain *chain, > ret = uvc_ctrl_add_mapping(chain, map); > > kfree(map->menu_info); > +free_name: > + if (ret) > + kfree(map->name); > free_map: > kfree(map); > > -- Ricardo Ribalda
