DF-5378 / fix.diff
Apply with: git apply -p0 (from /usr/src) or: patch -p0 < fix.diff Fix DF-5378: use-after-free of the UVC control graph in uvc_query_v4l2_ctrl()/uvc_query_v4l2_menu(). uvc_drv_detach() freed the entire control graph (uvc_drv_destroy_ctrl(), which also frees sc->ctrl itself) while the /dev/videoN cdev was still live (destroy_dev() only ran later, in uvc_v4l2_unreg()). An unprivileged VIDIOC_QUERYCTRL/VIDIOC_QUERYMENU issued around a device disconnect then: - took video->ctrl->mtx on freed memory (uvc_ctrls.c:1107) - walked the freed topo_nodes STAILQ (uvc_ctrls.c:1090) - wrote ctrl->info.cached_sub_info_idx (uvc_ctrls.c:1065) - strlcpy'd freed sub_info->v4l2_name into the user-visible queryctrl/querymenu name (uvc_ctrls.c:1001,1150) Fix layers: 1. uvc_drv_detach(): destroy the cdev (drains in-flight ioctls) and NULL video->ctrl under the ctrl lock BEFORE sc->data/sc->ctrl are freed; 2. uvc_query_v4l2_ctrl()/uvc_query_v4l2_menu(): bail out with ENODEV when video->ctrl is gone; 3. uvc_drv_destroy_ctrl(): take the ctrl mutex while tearing the graph down, and lockuninit() it (also fixes the missing uninit). --- sys/bus/u4b/uvc/uvc_drv.c +++ sys/bus/u4b/uvc/uvc_drv.c @@ -2456,6 +2456,7 @@ uint8_t i = 0; if (c) { + UVC_LOCK(&c->mtx); STAILQ_FOREACH_SAFE(topo_node, &c->topo_nodes, link, tmp) { for (i = 0; i < topo_node->controls_num; ++i) { struct uvc_control *ctrl = &topo_node->controls[i]; @@ -2469,6 +2470,8 @@ uvc_free_topo_node(topo_node); } + UVC_UNLOCK(&c->mtx); + lockuninit(&c->mtx); kfree(c, M_UVC); } } @@ -2499,6 +2502,20 @@ //delete sysfs //sysfs_video_del_device(sc->udev, sc->iface); + /* DF-5378: quiesce the /dev/videoN entry points FIRST. destroy_dev() + * drains in-flight ioctls, so after uvc_v4l2_unreg() no + * VIDIOC_QUERYCTRL/QUERYMENU can be running against the graphs we + * are about to free. video->ctrl is also NULLed under its lock so + * late query callers (uvc_ctrls.c) observe the teardown. */ + if (sc->video) { + if (sc->video->ctrl != NULL && sc->ctrl != NULL) { + UVC_LOCK(&sc->video->ctrl->mtx); + sc->video->ctrl = NULL; + UVC_UNLOCK(&sc->ctrl->mtx); + } + uvc_v4l2_unreg(sc->video); + } + /* destroy video hard code only one */ uvc_drv_destroy_video(sc->video); @@ -2507,8 +2524,6 @@ uvc_drv_destroy_ctrl(sc->ctrl); if (sc->video) { - /* entry close */ - uvc_v4l2_unreg(sc->video); /* data free */ kfree(sc->video, M_UVC); sc->video = NULL; --- sys/bus/u4b/uvc/uvc_ctrls.c +++ sys/bus/u4b/uvc/uvc_ctrls.c @@ -1104,6 +1104,9 @@ struct uvc_control *ctrl; int ret = EINVAL; + if (video->ctrl == NULL) + return (ENODEV); + UVC_LOCK(&video->ctrl->mtx); ctrl = uvc_search_control(video->ctrl, query->id); @@ -1128,6 +1131,9 @@ int id = qm->id; int index = qm->index; + if (video->ctrl == NULL) + return (ENODEV); + memset(qm, 0, sizeof(*qm)); qm->id = id; qm->index = index; |