Message ID | 20210408121041.6655-1-igormtorrente@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | media: em28xx: Fix race condition between open and init function | expand |
Hi Igor, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on linuxtv-media/master] [also build test WARNING on v5.12-rc6 next-20210408] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Igor-Matheus-Andrade-Torrente/media-em28xx-Fix-race-condition-between-open-and-init-function/20210408-201217 base: git://linuxtv.org/media_tree.git master config: x86_64-randconfig-a014-20210408 (attached as .config) compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 56ea2e2fdd691136d5e6631fa0e447173694b82c) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install x86_64 cross compiling tool for clang build # apt-get install binutils-x86-64-linux-gnu # https://github.com/0day-ci/linux/commit/e13d07271a1ee4cbd8ac421bf575a36f9d0e1008 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Igor-Matheus-Andrade-Torrente/media-em28xx-Fix-race-condition-between-open-and-init-function/20210408-201217 git checkout e13d07271a1ee4cbd8ac421bf575a36f9d0e1008 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): >> drivers/media/usb/em28xx/em28xx-video.c:2347:6: warning: no previous prototype for function 'em28xx_vdev_release' [-Wmissing-prototypes] void em28xx_vdev_release(struct video_device *vdev) ^ drivers/media/usb/em28xx/em28xx-video.c:2347:1: note: declare 'static' if the function is not intended to be used outside of this translation unit void em28xx_vdev_release(struct video_device *vdev) ^ static >> drivers/media/usb/em28xx/em28xx-video.c:2545:6: warning: no previous prototype for function 'em28xx_v4l2_dev_release' [-Wmissing-prototypes] void em28xx_v4l2_dev_release(struct v4l2_device *v4l2_dev) ^ drivers/media/usb/em28xx/em28xx-video.c:2545:1: note: declare 'static' if the function is not intended to be used outside of this translation unit void em28xx_v4l2_dev_release(struct v4l2_device *v4l2_dev) ^ static 2 warnings generated. vim +/em28xx_vdev_release +2347 drivers/media/usb/em28xx/em28xx-video.c 2346 > 2347 void em28xx_vdev_release(struct video_device *vdev) 2348 { 2349 #ifdef CONFIG_MEDIA_CONTROLLER 2350 int i; 2351 2352 for (i = 0; i < vdev->entity.num_pads; i++) 2353 kfree(&vdev->entity.pads[i]); 2354 #endif 2355 kfree(vdev); 2356 } 2357 2358 static const struct v4l2_file_operations em28xx_v4l_fops = { 2359 .owner = THIS_MODULE, 2360 .open = em28xx_v4l2_open, 2361 .release = em28xx_v4l2_close, 2362 .read = vb2_fop_read, 2363 .poll = vb2_fop_poll, 2364 .mmap = vb2_fop_mmap, 2365 .unlocked_ioctl = video_ioctl2, 2366 }; 2367 2368 static const struct v4l2_ioctl_ops video_ioctl_ops = { 2369 .vidioc_querycap = vidioc_querycap, 2370 .vidioc_enum_fmt_vid_cap = vidioc_enum_fmt_vid_cap, 2371 .vidioc_g_fmt_vid_cap = vidioc_g_fmt_vid_cap, 2372 .vidioc_try_fmt_vid_cap = vidioc_try_fmt_vid_cap, 2373 .vidioc_s_fmt_vid_cap = vidioc_s_fmt_vid_cap, 2374 .vidioc_g_fmt_vbi_cap = vidioc_g_fmt_vbi_cap, 2375 .vidioc_try_fmt_vbi_cap = vidioc_g_fmt_vbi_cap, 2376 .vidioc_s_fmt_vbi_cap = vidioc_g_fmt_vbi_cap, 2377 .vidioc_enum_framesizes = vidioc_enum_framesizes, 2378 .vidioc_enumaudio = vidioc_enumaudio, 2379 .vidioc_g_audio = vidioc_g_audio, 2380 .vidioc_s_audio = vidioc_s_audio, 2381 2382 .vidioc_reqbufs = vb2_ioctl_reqbufs, 2383 .vidioc_create_bufs = vb2_ioctl_create_bufs, 2384 .vidioc_prepare_buf = vb2_ioctl_prepare_buf, 2385 .vidioc_querybuf = vb2_ioctl_querybuf, 2386 .vidioc_qbuf = vb2_ioctl_qbuf, 2387 .vidioc_dqbuf = vb2_ioctl_dqbuf, 2388 2389 .vidioc_g_std = vidioc_g_std, 2390 .vidioc_querystd = vidioc_querystd, 2391 .vidioc_s_std = vidioc_s_std, 2392 .vidioc_g_parm = vidioc_g_parm, 2393 .vidioc_s_parm = vidioc_s_parm, 2394 .vidioc_enum_input = vidioc_enum_input, 2395 .vidioc_g_input = vidioc_g_input, 2396 .vidioc_s_input = vidioc_s_input, 2397 .vidioc_streamon = vb2_ioctl_streamon, 2398 .vidioc_streamoff = vb2_ioctl_streamoff, 2399 .vidioc_g_tuner = vidioc_g_tuner, 2400 .vidioc_s_tuner = vidioc_s_tuner, 2401 .vidioc_g_frequency = vidioc_g_frequency, 2402 .vidioc_s_frequency = vidioc_s_frequency, 2403 .vidioc_subscribe_event = v4l2_ctrl_subscribe_event, 2404 .vidioc_unsubscribe_event = v4l2_event_unsubscribe, 2405 #ifdef CONFIG_VIDEO_ADV_DEBUG 2406 .vidioc_g_chip_info = vidioc_g_chip_info, 2407 .vidioc_g_register = vidioc_g_register, 2408 .vidioc_s_register = vidioc_s_register, 2409 #endif 2410 }; 2411 2412 static const struct video_device em28xx_video_template = { 2413 .fops = &em28xx_v4l_fops, 2414 .ioctl_ops = &video_ioctl_ops, 2415 .release = em28xx_vdev_release, 2416 .tvnorms = V4L2_STD_ALL, 2417 }; 2418 2419 static const struct v4l2_file_operations radio_fops = { 2420 .owner = THIS_MODULE, 2421 .open = em28xx_v4l2_open, 2422 .release = em28xx_v4l2_close, 2423 .unlocked_ioctl = video_ioctl2, 2424 }; 2425 2426 static const struct v4l2_ioctl_ops radio_ioctl_ops = { 2427 .vidioc_querycap = vidioc_querycap, 2428 .vidioc_g_tuner = radio_g_tuner, 2429 .vidioc_s_tuner = radio_s_tuner, 2430 .vidioc_g_frequency = vidioc_g_frequency, 2431 .vidioc_s_frequency = vidioc_s_frequency, 2432 .vidioc_subscribe_event = v4l2_ctrl_subscribe_event, 2433 .vidioc_unsubscribe_event = v4l2_event_unsubscribe, 2434 #ifdef CONFIG_VIDEO_ADV_DEBUG 2435 .vidioc_g_chip_info = vidioc_g_chip_info, 2436 .vidioc_g_register = vidioc_g_register, 2437 .vidioc_s_register = vidioc_s_register, 2438 #endif 2439 }; 2440 2441 static struct video_device em28xx_radio_template = { 2442 .fops = &radio_fops, 2443 .ioctl_ops = &radio_ioctl_ops, 2444 .release = video_device_release_empty, 2445 }; 2446 2447 /* I2C possible address to saa7115, tvp5150, msp3400, tvaudio */ 2448 static unsigned short saa711x_addrs[] = { 2449 0x4a >> 1, 0x48 >> 1, /* SAA7111, SAA7111A and SAA7113 */ 2450 0x42 >> 1, 0x40 >> 1, /* SAA7114, SAA7115 and SAA7118 */ 2451 I2C_CLIENT_END }; 2452 2453 static unsigned short tvp5150_addrs[] = { 2454 0xb8 >> 1, 2455 0xba >> 1, 2456 I2C_CLIENT_END 2457 }; 2458 2459 static unsigned short msp3400_addrs[] = { 2460 0x80 >> 1, 2461 0x88 >> 1, 2462 I2C_CLIENT_END 2463 }; 2464 2465 /******************************** usb interface ******************************/ 2466 2467 static void em28xx_vdev_init(struct em28xx *dev, 2468 struct video_device *vfd, 2469 const struct video_device *template, 2470 const char *type_name) 2471 { 2472 *vfd = *template; 2473 vfd->v4l2_dev = dev->v4l2->v4l2_dev; 2474 vfd->lock = &dev->lock; 2475 if (dev->is_webcam) 2476 vfd->tvnorms = 0; 2477 2478 snprintf(vfd->name, sizeof(vfd->name), "%s %s", 2479 dev_name(&dev->intf->dev), type_name); 2480 2481 video_set_drvdata(vfd, dev); 2482 } 2483 2484 static void em28xx_tuner_setup(struct em28xx *dev, unsigned short tuner_addr) 2485 { 2486 struct em28xx_v4l2 *v4l2 = dev->v4l2; 2487 struct v4l2_device *v4l2_dev = v4l2->v4l2_dev; 2488 struct tuner_setup tun_setup; 2489 struct v4l2_frequency f; 2490 2491 memset(&tun_setup, 0, sizeof(tun_setup)); 2492 2493 tun_setup.mode_mask = T_ANALOG_TV | T_RADIO; 2494 tun_setup.tuner_callback = em28xx_tuner_callback; 2495 2496 if (dev->board.radio.type) { 2497 tun_setup.type = dev->board.radio.type; 2498 tun_setup.addr = dev->board.radio_addr; 2499 2500 v4l2_device_call_all(v4l2_dev, 2501 0, tuner, s_type_addr, &tun_setup); 2502 } 2503 2504 if (dev->tuner_type != TUNER_ABSENT && dev->tuner_type) { 2505 tun_setup.type = dev->tuner_type; 2506 tun_setup.addr = tuner_addr; 2507 2508 v4l2_device_call_all(v4l2_dev, 2509 0, tuner, s_type_addr, &tun_setup); 2510 } 2511 2512 if (dev->board.tda9887_conf) { 2513 struct v4l2_priv_tun_config tda9887_cfg; 2514 2515 tda9887_cfg.tuner = TUNER_TDA9887; 2516 tda9887_cfg.priv = &dev->board.tda9887_conf; 2517 2518 v4l2_device_call_all(v4l2_dev, 2519 0, tuner, s_config, &tda9887_cfg); 2520 } 2521 2522 if (dev->tuner_type == TUNER_XC2028) { 2523 struct v4l2_priv_tun_config xc2028_cfg; 2524 struct xc2028_ctrl ctl; 2525 2526 memset(&xc2028_cfg, 0, sizeof(xc2028_cfg)); 2527 memset(&ctl, 0, sizeof(ctl)); 2528 2529 em28xx_setup_xc3028(dev, &ctl); 2530 2531 xc2028_cfg.tuner = TUNER_XC2028; 2532 xc2028_cfg.priv = &ctl; 2533 2534 v4l2_device_call_all(v4l2_dev, 0, tuner, s_config, &xc2028_cfg); 2535 } 2536 2537 /* configure tuner */ 2538 f.tuner = 0; 2539 f.type = V4L2_TUNER_ANALOG_TV; 2540 f.frequency = 9076; /* just a magic number */ 2541 v4l2->frequency = f.frequency; 2542 v4l2_device_call_all(v4l2_dev, 0, tuner, s_frequency, &f); 2543 } 2544 > 2545 void em28xx_v4l2_dev_release(struct v4l2_device *v4l2_dev) 2546 { 2547 kfree(v4l2_dev); 2548 } 2549 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On 4/8/21 6:10 AM, Igor Matheus Andrade Torrente wrote: > Fixes a race condition - for lack of a more precise term - between > em28xx_v4l2_open and em28xx_v4l2_init, by detaching the v4l2_dev, > media_pad and vdev structs from the em28xx_v4l2, and managing the > lifetime of those objects more dynamicaly. > > The race happens when a thread[1] - containing the em28xx_v4l2_init() > code - calls the v4l2_mc_create_media_graph(), and it return a error, > if a thread[2] - running v4l2_open() - pass the verification point > and reaches the em28xx_v4l2_open() before the thread[1] finishes > the v4l2 subsystem deregistration, thread[1] will free all resources > before the em28xx_v4l2_open() can process their things, > because the em28xx_v4l2_init() has the dev->lock. And all this lead > the thread[2] to cause a user-after-free. > Have you tried this patch with em28xx device? You will have to take into account the dependencies between the subdevs using the v4l2_dev. Also try rmmod invidual drivers - what happens if you were to rmmod a subdev driver? With v4l2_dev is not embedded in v4l2, this could open up memory leaks or user-after-frees. > Reported-and-tested-by: syzbot+b2391895514ed9ef4a8e@syzkaller.appspotmail.com > Signed-off-by: Igor Matheus Andrade Torrente <igormtorrente@gmail.com> > --- > drivers/media/usb/em28xx/em28xx-camera.c | 4 +- > drivers/media/usb/em28xx/em28xx-video.c | 188 ++++++++++++++--------- > drivers/media/usb/em28xx/em28xx.h | 6 +- > 3 files changed, 123 insertions(+), 75 deletions(-) > > diff --git a/drivers/media/usb/em28xx/em28xx-camera.c b/drivers/media/usb/em28xx/em28xx-camera.c > index d1e66b503f4d..436c5a8cbbb6 100644 > --- a/drivers/media/usb/em28xx/em28xx-camera.c > +++ b/drivers/media/usb/em28xx/em28xx-camera.c > @@ -340,7 +340,7 @@ int em28xx_init_camera(struct em28xx *dev) > v4l2->sensor_xtal = 4300000; > pdata.xtal = v4l2->sensor_xtal; > if (NULL == > - v4l2_i2c_new_subdev_board(&v4l2->v4l2_dev, adap, > + v4l2_i2c_new_subdev_board(v4l2->v4l2_dev, adap, > &mt9v011_info, NULL)) > return -ENODEV; > v4l2->vinmode = EM28XX_VINMODE_RGB8_GRBG; > @@ -394,7 +394,7 @@ int em28xx_init_camera(struct em28xx *dev) > v4l2->sensor_yres = 480; > > subdev = > - v4l2_i2c_new_subdev_board(&v4l2->v4l2_dev, adap, > + v4l2_i2c_new_subdev_board(v4l2->v4l2_dev, adap, > &ov2640_info, NULL); > if (!subdev) > return -ENODEV; > diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c > index 6b84c3413e83..e1febb2bf06b 100644 > --- a/drivers/media/usb/em28xx/em28xx-video.c > +++ b/drivers/media/usb/em28xx/em28xx-video.c > @@ -184,7 +184,7 @@ static int em28xx_vbi_supported(struct em28xx *dev) > */ > static void em28xx_wake_i2c(struct em28xx *dev) > { > - struct v4l2_device *v4l2_dev = &dev->v4l2->v4l2_dev; > + struct v4l2_device *v4l2_dev = dev->v4l2->v4l2_dev; > > v4l2_device_call_all(v4l2_dev, 0, core, reset, 0); > v4l2_device_call_all(v4l2_dev, 0, video, s_routing, > @@ -974,9 +974,17 @@ static void em28xx_v4l2_create_entities(struct em28xx *dev) > struct em28xx_v4l2 *v4l2 = dev->v4l2; > int ret, i; > > + v4l2->video_pad = kzalloc(sizeof(*v4l2->video_pad), GFP_KERNEL); > + if (!v4l2->video_pad) { > + dev_err(&dev->intf->dev, > + "failed to allocate video pad memory!\n"); > + v4l2->vdev->entity.num_pads = 0; > + return; > + } > + > /* Initialize Video, VBI and Radio pads */ > - v4l2->video_pad.flags = MEDIA_PAD_FL_SINK; > - ret = media_entity_pads_init(&v4l2->vdev.entity, 1, &v4l2->video_pad); > + v4l2->video_pad->flags = MEDIA_PAD_FL_SINK; > + ret = media_entity_pads_init(&v4l2->vdev->entity, 1, v4l2->video_pad); > if (ret < 0) > dev_err(&dev->intf->dev, > "failed to initialize video media entity!\n"); > @@ -1132,11 +1140,11 @@ int em28xx_start_analog_streaming(struct vb2_queue *vq, unsigned int count) > f.type = V4L2_TUNER_RADIO; > else > f.type = V4L2_TUNER_ANALOG_TV; > - v4l2_device_call_all(&v4l2->v4l2_dev, > + v4l2_device_call_all(v4l2->v4l2_dev, > 0, tuner, s_frequency, &f); > > /* Enable video stream at TV decoder */ > - v4l2_device_call_all(&v4l2->v4l2_dev, 0, video, s_stream, 1); > + v4l2_device_call_all(v4l2->v4l2_dev, 0, video, s_stream, 1); > } > > v4l2->streaming_users++; > @@ -1157,7 +1165,7 @@ static void em28xx_stop_streaming(struct vb2_queue *vq) > > if (v4l2->streaming_users-- == 1) { > /* Disable video stream at TV decoder */ > - v4l2_device_call_all(&v4l2->v4l2_dev, 0, video, s_stream, 0); > + v4l2_device_call_all(v4l2->v4l2_dev, 0, video, s_stream, 0); > > /* Last active user, so shutdown all the URBS */ > em28xx_uninit_usb_xfer(dev, EM28XX_ANALOG_MODE); > @@ -1192,7 +1200,7 @@ void em28xx_stop_vbi_streaming(struct vb2_queue *vq) > > if (v4l2->streaming_users-- == 1) { > /* Disable video stream at TV decoder */ > - v4l2_device_call_all(&v4l2->v4l2_dev, 0, video, s_stream, 0); > + v4l2_device_call_all(v4l2->v4l2_dev, 0, video, s_stream, 0); > > /* Last active user, so shutdown all the URBS */ > em28xx_uninit_usb_xfer(dev, EM28XX_ANALOG_MODE); > @@ -1286,7 +1294,7 @@ static int em28xx_vb2_setup(struct em28xx *dev) > > static void video_mux(struct em28xx *dev, int index) > { > - struct v4l2_device *v4l2_dev = &dev->v4l2->v4l2_dev; > + struct v4l2_device *v4l2_dev = dev->v4l2->v4l2_dev; > > dev->ctl_input = index; > dev->ctl_ainput = INPUT(index)->amux; > @@ -1565,7 +1573,7 @@ static int vidioc_querystd(struct file *file, void *priv, v4l2_std_id *norm) > { > struct em28xx *dev = video_drvdata(file); > > - v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, video, querystd, norm); > + v4l2_device_call_all(dev->v4l2->v4l2_dev, 0, video, querystd, norm); > > return 0; > } > @@ -1596,7 +1604,7 @@ static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id norm) > &v4l2->hscale, &v4l2->vscale); > > em28xx_resolution_set(dev); > - v4l2_device_call_all(&v4l2->v4l2_dev, 0, video, s_std, v4l2->norm); > + v4l2_device_call_all(v4l2->v4l2_dev, 0, video, s_std, v4l2->norm); > > return 0; > } > @@ -1616,7 +1624,7 @@ static int vidioc_g_parm(struct file *file, void *priv, > p->parm.capture.readbuffers = EM28XX_MIN_BUF; > p->parm.capture.capability = V4L2_CAP_TIMEPERFRAME; > if (dev->is_webcam) { > - rc = v4l2_device_call_until_err(&v4l2->v4l2_dev, 0, > + rc = v4l2_device_call_until_err(v4l2->v4l2_dev, 0, > video, g_frame_interval, &ival); > if (!rc) > p->parm.capture.timeperframe = ival.interval; > @@ -1648,7 +1656,7 @@ static int vidioc_s_parm(struct file *file, void *priv, > memset(&p->parm, 0, sizeof(p->parm)); > p->parm.capture.readbuffers = EM28XX_MIN_BUF; > p->parm.capture.capability = V4L2_CAP_TIMEPERFRAME; > - rc = v4l2_device_call_until_err(&dev->v4l2->v4l2_dev, 0, > + rc = v4l2_device_call_until_err(dev->v4l2->v4l2_dev, 0, > video, s_frame_interval, &ival); > if (!rc) > p->parm.capture.timeperframe = ival.interval; > @@ -1675,7 +1683,7 @@ static int vidioc_enum_input(struct file *file, void *priv, > if (INPUT(n)->type == EM28XX_VMUX_TELEVISION) > i->type = V4L2_INPUT_TYPE_TUNER; > > - i->std = dev->v4l2->vdev.tvnorms; > + i->std = dev->v4l2->vdev->tvnorms; > /* webcams do not have the STD API */ > if (dev->is_webcam) > i->capabilities = 0; > @@ -1839,7 +1847,7 @@ static int vidioc_g_tuner(struct file *file, void *priv, > > strscpy(t->name, "Tuner", sizeof(t->name)); > > - v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, tuner, g_tuner, t); > + v4l2_device_call_all(dev->v4l2->v4l2_dev, 0, tuner, g_tuner, t); > return 0; > } > > @@ -1851,7 +1859,7 @@ static int vidioc_s_tuner(struct file *file, void *priv, > if (t->index != 0) > return -EINVAL; > > - v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, tuner, s_tuner, t); > + v4l2_device_call_all(dev->v4l2->v4l2_dev, 0, tuner, s_tuner, t); > return 0; > } > > @@ -1878,8 +1886,8 @@ static int vidioc_s_frequency(struct file *file, void *priv, > if (f->tuner != 0) > return -EINVAL; > > - v4l2_device_call_all(&v4l2->v4l2_dev, 0, tuner, s_frequency, f); > - v4l2_device_call_all(&v4l2->v4l2_dev, 0, tuner, g_frequency, &new_freq); > + v4l2_device_call_all(v4l2->v4l2_dev, 0, tuner, s_frequency, f); > + v4l2_device_call_all(v4l2->v4l2_dev, 0, tuner, g_frequency, &new_freq); > v4l2->frequency = new_freq.frequency; > > return 0; > @@ -1897,7 +1905,7 @@ static int vidioc_g_chip_info(struct file *file, void *priv, > strscpy(chip->name, "ac97", sizeof(chip->name)); > else > strscpy(chip->name, > - dev->v4l2->v4l2_dev.name, sizeof(chip->name)); > + dev->v4l2->v4l2_dev->name, sizeof(chip->name)); > return 0; > } > > @@ -2095,7 +2103,7 @@ static int radio_g_tuner(struct file *file, void *priv, > > strscpy(t->name, "Radio", sizeof(t->name)); > > - v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, tuner, g_tuner, t); > + v4l2_device_call_all(dev->v4l2->v4l2_dev, 0, tuner, g_tuner, t); > > return 0; > } > @@ -2108,7 +2116,7 @@ static int radio_s_tuner(struct file *file, void *priv, > if (t->index != 0) > return -EINVAL; > > - v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, tuner, s_tuner, t); > + v4l2_device_call_all(dev->v4l2->v4l2_dev, 0, tuner, s_tuner, t); > > return 0; > } > @@ -2160,6 +2168,11 @@ static int em28xx_v4l2_open(struct file *filp) > if (mutex_lock_interruptible(&dev->lock)) > return -ERESTARTSYS; > > + if (!dev->v4l2) { > + mutex_unlock(&dev->lock); > + return -ENODEV; > + } > + > ret = v4l2_fh_open(filp); > if (ret) { > dev_err(&dev->intf->dev, > @@ -2184,7 +2197,7 @@ static int em28xx_v4l2_open(struct file *filp) > > if (vdev->vfl_type == VFL_TYPE_RADIO) { > em28xx_videodbg("video_open: setting radio device\n"); > - v4l2_device_call_all(&v4l2->v4l2_dev, 0, tuner, s_radio); > + v4l2_device_call_all(v4l2->v4l2_dev, 0, tuner, s_radio); > } > > kref_get(&dev->ref); > @@ -2222,7 +2235,7 @@ static int em28xx_v4l2_fini(struct em28xx *dev) > > mutex_lock(&dev->lock); > > - v4l2_device_disconnect(&v4l2->v4l2_dev); > + v4l2_device_disconnect(v4l2->v4l2_dev); > > em28xx_uninit_usb_xfer(dev, EM28XX_ANALOG_MODE); > > @@ -2238,14 +2251,15 @@ static int em28xx_v4l2_fini(struct em28xx *dev) > video_device_node_name(&v4l2->vbi_dev)); > video_unregister_device(&v4l2->vbi_dev); > } > - if (video_is_registered(&v4l2->vdev)) { > + if (video_is_registered(v4l2->vdev)) { > dev_info(&dev->intf->dev, "V4L2 device %s deregistered\n", > - video_device_node_name(&v4l2->vdev)); > - video_unregister_device(&v4l2->vdev); > + video_device_node_name(v4l2->vdev)); > + video_unregister_device(v4l2->vdev); > } > > v4l2_ctrl_handler_free(&v4l2->ctrl_handler); > - v4l2_device_unregister(&v4l2->v4l2_dev); > + v4l2_device_unregister(v4l2->v4l2_dev); > + v4l2_device_put(v4l2->v4l2_dev); > > kref_put(&v4l2->ref, em28xx_free_v4l2); > > @@ -2305,7 +2319,7 @@ static int em28xx_v4l2_close(struct file *filp) > goto exit; > > /* Save some power by putting tuner to sleep */ > - v4l2_device_call_all(&v4l2->v4l2_dev, 0, tuner, standby); > + v4l2_device_call_all(v4l2->v4l2_dev, 0, tuner, standby); > > /* do this before setting alternate! */ > em28xx_set_mode(dev, EM28XX_SUSPEND); > @@ -2330,6 +2344,17 @@ static int em28xx_v4l2_close(struct file *filp) > return 0; > } > > +void em28xx_vdev_release(struct video_device *vdev) > +{ > +#ifdef CONFIG_MEDIA_CONTROLLER > + int i; > + > + for (i = 0; i < vdev->entity.num_pads; i++) > + kfree(&vdev->entity.pads[i]); > +#endif > + kfree(vdev); > +} > + > static const struct v4l2_file_operations em28xx_v4l_fops = { > .owner = THIS_MODULE, > .open = em28xx_v4l2_open, > @@ -2387,7 +2412,7 @@ static const struct v4l2_ioctl_ops video_ioctl_ops = { > static const struct video_device em28xx_video_template = { > .fops = &em28xx_v4l_fops, > .ioctl_ops = &video_ioctl_ops, > - .release = video_device_release_empty, > + .release = em28xx_vdev_release, > .tvnorms = V4L2_STD_ALL, > }; > > @@ -2445,7 +2470,7 @@ static void em28xx_vdev_init(struct em28xx *dev, > const char *type_name) > { > *vfd = *template; > - vfd->v4l2_dev = &dev->v4l2->v4l2_dev; > + vfd->v4l2_dev = dev->v4l2->v4l2_dev; > vfd->lock = &dev->lock; > if (dev->is_webcam) > vfd->tvnorms = 0; > @@ -2459,7 +2484,7 @@ static void em28xx_vdev_init(struct em28xx *dev, > static void em28xx_tuner_setup(struct em28xx *dev, unsigned short tuner_addr) > { > struct em28xx_v4l2 *v4l2 = dev->v4l2; > - struct v4l2_device *v4l2_dev = &v4l2->v4l2_dev; > + struct v4l2_device *v4l2_dev = v4l2->v4l2_dev; > struct tuner_setup tun_setup; > struct v4l2_frequency f; > > @@ -2517,6 +2542,11 @@ static void em28xx_tuner_setup(struct em28xx *dev, unsigned short tuner_addr) > v4l2_device_call_all(v4l2_dev, 0, tuner, s_frequency, &f); > } > > +void em28xx_v4l2_dev_release(struct v4l2_device *v4l2_dev) > +{ > + kfree(v4l2_dev); > +} > + > static int em28xx_v4l2_init(struct em28xx *dev) > { > u8 val; > @@ -2541,26 +2571,35 @@ static int em28xx_v4l2_init(struct em28xx *dev) > > v4l2 = kzalloc(sizeof(*v4l2), GFP_KERNEL); > if (!v4l2) { > - mutex_unlock(&dev->lock); > - return -ENOMEM; > + ret = -ENOMEM; > + goto v4l2_error; > } > + > kref_init(&v4l2->ref); > v4l2->dev = dev; > dev->v4l2 = v4l2; > > + v4l2->v4l2_dev = kzalloc(sizeof(*v4l2->v4l2_dev), GFP_KERNEL); > + if (!v4l2->v4l2_dev) { > + ret = -ENOMEM; > + goto v4l2_dev_error; > + } > + > + v4l2->v4l2_dev->release = em28xx_v4l2_dev_release; > + > #ifdef CONFIG_MEDIA_CONTROLLER > - v4l2->v4l2_dev.mdev = dev->media_dev; > + v4l2->v4l2_dev->mdev = dev->media_dev; > #endif > - ret = v4l2_device_register(&dev->intf->dev, &v4l2->v4l2_dev); > + ret = v4l2_device_register(&dev->intf->dev, v4l2->v4l2_dev); > if (ret < 0) { > dev_err(&dev->intf->dev, > "Call to v4l2_device_register() failed!\n"); > - goto err; > + goto v4l2_device_register_error; > } > > hdl = &v4l2->ctrl_handler; > v4l2_ctrl_handler_init(hdl, 8); > - v4l2->v4l2_dev.ctrl_handler = hdl; > + v4l2->v4l2_dev->ctrl_handler = hdl; > > if (dev->is_webcam) > v4l2->progressive = true; > @@ -2575,22 +2614,22 @@ static int em28xx_v4l2_init(struct em28xx *dev) > /* request some modules */ > > if (dev->has_msp34xx) > - v4l2_i2c_new_subdev(&v4l2->v4l2_dev, > + v4l2_i2c_new_subdev(v4l2->v4l2_dev, > &dev->i2c_adap[dev->def_i2c_bus], > "msp3400", 0, msp3400_addrs); > > if (dev->board.decoder == EM28XX_SAA711X) > - v4l2_i2c_new_subdev(&v4l2->v4l2_dev, > + v4l2_i2c_new_subdev(v4l2->v4l2_dev, > &dev->i2c_adap[dev->def_i2c_bus], > "saa7115_auto", 0, saa711x_addrs); > > if (dev->board.decoder == EM28XX_TVP5150) > - v4l2_i2c_new_subdev(&v4l2->v4l2_dev, > + v4l2_i2c_new_subdev(v4l2->v4l2_dev, > &dev->i2c_adap[dev->def_i2c_bus], > "tvp5150", 0, tvp5150_addrs); > > if (dev->board.adecoder == EM28XX_TVAUDIO) > - v4l2_i2c_new_subdev(&v4l2->v4l2_dev, > + v4l2_i2c_new_subdev(v4l2->v4l2_dev, > &dev->i2c_adap[dev->def_i2c_bus], > "tvaudio", dev->board.tvaudio_addr, NULL); > > @@ -2601,13 +2640,13 @@ static int em28xx_v4l2_init(struct em28xx *dev) > int has_demod = (dev->board.tda9887_conf & TDA9887_PRESENT); > > if (dev->board.radio.type) > - v4l2_i2c_new_subdev(&v4l2->v4l2_dev, > + v4l2_i2c_new_subdev(v4l2->v4l2_dev, > &dev->i2c_adap[dev->def_i2c_bus], > "tuner", dev->board.radio_addr, > NULL); Add null check for v4l2_i2c_new_subdev() and error handling. It was okay check error prior to this change to allocating v4l2_dev. Now this has to be handled as a error leg. > > if (has_demod) > - v4l2_i2c_new_subdev(&v4l2->v4l2_dev, > + v4l2_i2c_new_subdev(v4l2->v4l2_dev, > &dev->i2c_adap[dev->def_i2c_bus], > "tuner", 0, > v4l2_i2c_tuner_addrs(ADDRS_DEMOD)); Same here: Add null check for v4l2_i2c_new_subdev() and error handling. It was okay check error prior to this change to allocating v4l2_dev. Now this has to be handled as a error leg. > @@ -2616,7 +2655,7 @@ static int em28xx_v4l2_init(struct em28xx *dev) > has_demod ? ADDRS_TV_WITH_DEMOD : ADDRS_TV; > struct v4l2_subdev *sd; > > - sd = v4l2_i2c_new_subdev(&v4l2->v4l2_dev, > + sd = v4l2_i2c_new_subdev(v4l2->v4l2_dev, > &dev->i2c_adap[dev->def_i2c_bus], > "tuner", 0, > v4l2_i2c_tuner_addrs(type)); > @@ -2624,7 +2663,7 @@ static int em28xx_v4l2_init(struct em28xx *dev) > if (sd) > tuner_addr = v4l2_i2c_subdev_addr(sd); Add null check for v4l2_i2c_new_subdev() and error handling. It was okay check error prior to this change to allocating v4l2_dev. Now this has to be handled as a error leg. > } else { > - v4l2_i2c_new_subdev(&v4l2->v4l2_dev, > + v4l2_i2c_new_subdev(v4l2->v4l2_dev, > &dev->i2c_adap[dev->def_i2c_bus], > "tuner", tuner_addr, NULL); Add null check for v4l2_i2c_new_subdev() and error handling. It was okay check error prior to this change to allocating v4l2_dev. Now this has to be handled as a error leg. > } > @@ -2686,7 +2725,7 @@ static int em28xx_v4l2_init(struct em28xx *dev) > > /* set default norm */ > v4l2->norm = V4L2_STD_PAL; > - v4l2_device_call_all(&v4l2->v4l2_dev, 0, video, s_std, v4l2->norm); > + v4l2_device_call_all(v4l2->v4l2_dev, 0, video, s_std, v4l2->norm); > v4l2->interlaced_fieldmode = EM28XX_INTERLACED_DEFAULT; > > /* Analog specific initialization */ > @@ -2756,40 +2795,45 @@ static int em28xx_v4l2_init(struct em28xx *dev) > goto unregister_dev; > > /* allocate and fill video video_device struct */ > - em28xx_vdev_init(dev, &v4l2->vdev, &em28xx_video_template, "video"); > + v4l2->vdev = kzalloc(sizeof(*v4l2->vdev), GFP_KERNEL); > + if (!v4l2->vdev) { > + ret = -ENOMEM; > + goto unregister_dev; > + } > + > + em28xx_vdev_init(dev, v4l2->vdev, &em28xx_video_template, "video"); > mutex_init(&v4l2->vb_queue_lock); > mutex_init(&v4l2->vb_vbi_queue_lock); > - v4l2->vdev.queue = &v4l2->vb_vidq; > - v4l2->vdev.queue->lock = &v4l2->vb_queue_lock; > - v4l2->vdev.device_caps = V4L2_CAP_READWRITE | V4L2_CAP_VIDEO_CAPTURE | > + v4l2->vdev->queue = &v4l2->vb_vidq; > + v4l2->vdev->queue->lock = &v4l2->vb_queue_lock; > + v4l2->vdev->device_caps = V4L2_CAP_READWRITE | V4L2_CAP_VIDEO_CAPTURE | > V4L2_CAP_STREAMING; > if (dev->int_audio_type != EM28XX_INT_AUDIO_NONE) > - v4l2->vdev.device_caps |= V4L2_CAP_AUDIO; > + v4l2->vdev->device_caps |= V4L2_CAP_AUDIO; > if (dev->tuner_type != TUNER_ABSENT) > - v4l2->vdev.device_caps |= V4L2_CAP_TUNER; > - > + v4l2->vdev->device_caps |= V4L2_CAP_TUNER; > > /* disable inapplicable ioctls */ > if (dev->is_webcam) { > - v4l2_disable_ioctl(&v4l2->vdev, VIDIOC_QUERYSTD); > - v4l2_disable_ioctl(&v4l2->vdev, VIDIOC_G_STD); > - v4l2_disable_ioctl(&v4l2->vdev, VIDIOC_S_STD); > + v4l2_disable_ioctl(v4l2->vdev, VIDIOC_QUERYSTD); > + v4l2_disable_ioctl(v4l2->vdev, VIDIOC_G_STD); > + v4l2_disable_ioctl(v4l2->vdev, VIDIOC_S_STD); > } else { > - v4l2_disable_ioctl(&v4l2->vdev, VIDIOC_S_PARM); > + v4l2_disable_ioctl(v4l2->vdev, VIDIOC_S_PARM); > } > if (dev->tuner_type == TUNER_ABSENT) { > - v4l2_disable_ioctl(&v4l2->vdev, VIDIOC_G_TUNER); > - v4l2_disable_ioctl(&v4l2->vdev, VIDIOC_S_TUNER); > - v4l2_disable_ioctl(&v4l2->vdev, VIDIOC_G_FREQUENCY); > - v4l2_disable_ioctl(&v4l2->vdev, VIDIOC_S_FREQUENCY); > + v4l2_disable_ioctl(v4l2->vdev, VIDIOC_G_TUNER); > + v4l2_disable_ioctl(v4l2->vdev, VIDIOC_S_TUNER); > + v4l2_disable_ioctl(v4l2->vdev, VIDIOC_G_FREQUENCY); > + v4l2_disable_ioctl(v4l2->vdev, VIDIOC_S_FREQUENCY); > } > if (dev->int_audio_type == EM28XX_INT_AUDIO_NONE) { > - v4l2_disable_ioctl(&v4l2->vdev, VIDIOC_G_AUDIO); > - v4l2_disable_ioctl(&v4l2->vdev, VIDIOC_S_AUDIO); > + v4l2_disable_ioctl(v4l2->vdev, VIDIOC_G_AUDIO); > + v4l2_disable_ioctl(v4l2->vdev, VIDIOC_S_AUDIO); > } > > /* register v4l2 video video_device */ > - ret = video_register_device(&v4l2->vdev, VFL_TYPE_VIDEO, > + ret = video_register_device(v4l2->vdev, VFL_TYPE_VIDEO, > video_nr[dev->devno]); > if (ret) { > dev_err(&dev->intf->dev, > @@ -2863,7 +2907,7 @@ static int em28xx_v4l2_init(struct em28xx *dev) > > dev_info(&dev->intf->dev, > "V4L2 video device registered as %s\n", > - video_device_node_name(&v4l2->vdev)); > + video_device_node_name(v4l2->vdev)); > > if (video_is_registered(&v4l2->vbi_dev)) > dev_info(&dev->intf->dev, > @@ -2871,7 +2915,7 @@ static int em28xx_v4l2_init(struct em28xx *dev) > video_device_node_name(&v4l2->vbi_dev)); > > /* Save some power by putting tuner to sleep */ > - v4l2_device_call_all(&v4l2->v4l2_dev, 0, tuner, standby); > + v4l2_device_call_all(v4l2->v4l2_dev, 0, tuner, standby); > > /* initialize videobuf2 stuff */ > em28xx_vb2_setup(dev); > @@ -2897,18 +2941,22 @@ static int em28xx_v4l2_init(struct em28xx *dev) > video_device_node_name(&v4l2->vbi_dev)); > video_unregister_device(&v4l2->vbi_dev); > } > - if (video_is_registered(&v4l2->vdev)) { > + if (video_is_registered(v4l2->vdev)) { > dev_info(&dev->intf->dev, > "V4L2 device %s deregistered\n", > - video_device_node_name(&v4l2->vdev)); > - video_unregister_device(&v4l2->vdev); > + video_device_node_name(v4l2->vdev)); > + video_unregister_device(v4l2->vdev); > } > > v4l2_ctrl_handler_free(&v4l2->ctrl_handler); > - v4l2_device_unregister(&v4l2->v4l2_dev); > -err: > + v4l2_device_unregister(v4l2->v4l2_dev); > + > +v4l2_device_register_error: > + v4l2_device_put(v4l2->v4l2_dev); > +v4l2_dev_error: > dev->v4l2 = NULL; > kref_put(&v4l2->ref, em28xx_free_v4l2); > +v4l2_error: > mutex_unlock(&dev->lock); > return ret; > } > diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h > index 6648e11f1271..dbcc297b5a0d 100644 > --- a/drivers/media/usb/em28xx/em28xx.h > +++ b/drivers/media/usb/em28xx/em28xx.h > @@ -552,10 +552,10 @@ struct em28xx_v4l2 { > struct kref ref; > struct em28xx *dev; > > - struct v4l2_device v4l2_dev; > + struct v4l2_device *v4l2_dev; > struct v4l2_ctrl_handler ctrl_handler; > > - struct video_device vdev; > + struct video_device *vdev; > struct video_device vbi_dev; > struct video_device radio_dev; > > @@ -601,7 +601,7 @@ struct em28xx_v4l2 { > unsigned int field_count; > > #ifdef CONFIG_MEDIA_CONTROLLER > - struct media_pad video_pad, vbi_pad; > + struct media_pad *video_pad, vbi_pad; > struct media_entity *decoder; > #endif > }; > thanks, -- Shuah
On 4/8/21 2:37 PM, Shuah Khan wrote: > On 4/8/21 6:10 AM, Igor Matheus Andrade Torrente wrote: >> Fixes a race condition - for lack of a more precise term - between >> em28xx_v4l2_open and em28xx_v4l2_init, by detaching the v4l2_dev, >> media_pad and vdev structs from the em28xx_v4l2, and managing the >> lifetime of those objects more dynamicaly. >> >> The race happens when a thread[1] - containing the em28xx_v4l2_init() >> code - calls the v4l2_mc_create_media_graph(), and it return a error, >> if a thread[2] - running v4l2_open() - pass the verification point >> and reaches the em28xx_v4l2_open() before the thread[1] finishes >> the v4l2 subsystem deregistration, thread[1] will free all resources >> before the em28xx_v4l2_open() can process their things, >> because the em28xx_v4l2_init() has the dev->lock. And all this lead >> the thread[2] to cause a user-after-free. >> > > Have you tried this patch with em28xx device? You will have to take into > account the dependencies between the subdevs using the v4l2_dev. > No, I didn't because I don't have the device. I was using the syzbot .c that produces this use-after-free. I will try to modify the .c to test a complete initialization. Actually, I forgot to put RFC alongside [PATCH]. > Also try rmmod invidual drivers - what happens if you were to rmmod a > subdev driver? With v4l2_dev is not embedded in v4l2, this could open > up memory leaks or user-after-frees. > OK. Let me see what I can do. >> Reported-and-tested-by: >> syzbot+b2391895514ed9ef4a8e@syzkaller.appspotmail.com >> Signed-off-by: Igor Matheus Andrade Torrente <igormtorrente@gmail.com> >> --- >> drivers/media/usb/em28xx/em28xx-camera.c | 4 +- >> drivers/media/usb/em28xx/em28xx-video.c | 188 ++++++++++++++--------- >> drivers/media/usb/em28xx/em28xx.h | 6 +- >> 3 files changed, 123 insertions(+), 75 deletions(-) >> >> diff --git a/drivers/media/usb/em28xx/em28xx-camera.c >> b/drivers/media/usb/em28xx/em28xx-camera.c >> index d1e66b503f4d..436c5a8cbbb6 100644 >> --- a/drivers/media/usb/em28xx/em28xx-camera.c >> +++ b/drivers/media/usb/em28xx/em28xx-camera.c >> @@ -340,7 +340,7 @@ int em28xx_init_camera(struct em28xx *dev) >> v4l2->sensor_xtal = 4300000; >> pdata.xtal = v4l2->sensor_xtal; >> if (NULL == >> - v4l2_i2c_new_subdev_board(&v4l2->v4l2_dev, adap, >> + v4l2_i2c_new_subdev_board(v4l2->v4l2_dev, adap, >> &mt9v011_info, NULL)) >> return -ENODEV; >> v4l2->vinmode = EM28XX_VINMODE_RGB8_GRBG; >> @@ -394,7 +394,7 @@ int em28xx_init_camera(struct em28xx *dev) >> v4l2->sensor_yres = 480; >> subdev = >> - v4l2_i2c_new_subdev_board(&v4l2->v4l2_dev, adap, >> + v4l2_i2c_new_subdev_board(v4l2->v4l2_dev, adap, >> &ov2640_info, NULL); >> if (!subdev) >> return -ENODEV; >> diff --git a/drivers/media/usb/em28xx/em28xx-video.c >> b/drivers/media/usb/em28xx/em28xx-video.c >> index 6b84c3413e83..e1febb2bf06b 100644 >> --- a/drivers/media/usb/em28xx/em28xx-video.c >> +++ b/drivers/media/usb/em28xx/em28xx-video.c >> @@ -184,7 +184,7 @@ static int em28xx_vbi_supported(struct em28xx *dev) >> */ >> static void em28xx_wake_i2c(struct em28xx *dev) >> { >> - struct v4l2_device *v4l2_dev = &dev->v4l2->v4l2_dev; >> + struct v4l2_device *v4l2_dev = dev->v4l2->v4l2_dev; >> v4l2_device_call_all(v4l2_dev, 0, core, reset, 0); >> v4l2_device_call_all(v4l2_dev, 0, video, s_routing, >> @@ -974,9 +974,17 @@ static void em28xx_v4l2_create_entities(struct >> em28xx *dev) >> struct em28xx_v4l2 *v4l2 = dev->v4l2; >> int ret, i; >> + v4l2->video_pad = kzalloc(sizeof(*v4l2->video_pad), GFP_KERNEL); >> + if (!v4l2->video_pad) { >> + dev_err(&dev->intf->dev, >> + "failed to allocate video pad memory!\n"); >> + v4l2->vdev->entity.num_pads = 0; >> + return; >> + } >> + >> /* Initialize Video, VBI and Radio pads */ >> - v4l2->video_pad.flags = MEDIA_PAD_FL_SINK; >> - ret = media_entity_pads_init(&v4l2->vdev.entity, 1, >> &v4l2->video_pad); >> + v4l2->video_pad->flags = MEDIA_PAD_FL_SINK; >> + ret = media_entity_pads_init(&v4l2->vdev->entity, 1, >> v4l2->video_pad); >> if (ret < 0) >> dev_err(&dev->intf->dev, >> "failed to initialize video media entity!\n"); >> @@ -1132,11 +1140,11 @@ int em28xx_start_analog_streaming(struct >> vb2_queue *vq, unsigned int count) >> f.type = V4L2_TUNER_RADIO; >> else >> f.type = V4L2_TUNER_ANALOG_TV; >> - v4l2_device_call_all(&v4l2->v4l2_dev, >> + v4l2_device_call_all(v4l2->v4l2_dev, >> 0, tuner, s_frequency, &f); >> /* Enable video stream at TV decoder */ >> - v4l2_device_call_all(&v4l2->v4l2_dev, 0, video, s_stream, 1); >> + v4l2_device_call_all(v4l2->v4l2_dev, 0, video, s_stream, 1); >> } >> v4l2->streaming_users++; >> @@ -1157,7 +1165,7 @@ static void em28xx_stop_streaming(struct >> vb2_queue *vq) >> if (v4l2->streaming_users-- == 1) { >> /* Disable video stream at TV decoder */ >> - v4l2_device_call_all(&v4l2->v4l2_dev, 0, video, s_stream, 0); >> + v4l2_device_call_all(v4l2->v4l2_dev, 0, video, s_stream, 0); >> /* Last active user, so shutdown all the URBS */ >> em28xx_uninit_usb_xfer(dev, EM28XX_ANALOG_MODE); >> @@ -1192,7 +1200,7 @@ void em28xx_stop_vbi_streaming(struct vb2_queue >> *vq) >> if (v4l2->streaming_users-- == 1) { >> /* Disable video stream at TV decoder */ >> - v4l2_device_call_all(&v4l2->v4l2_dev, 0, video, s_stream, 0); >> + v4l2_device_call_all(v4l2->v4l2_dev, 0, video, s_stream, 0); >> /* Last active user, so shutdown all the URBS */ >> em28xx_uninit_usb_xfer(dev, EM28XX_ANALOG_MODE); >> @@ -1286,7 +1294,7 @@ static int em28xx_vb2_setup(struct em28xx *dev) >> static void video_mux(struct em28xx *dev, int index) >> { >> - struct v4l2_device *v4l2_dev = &dev->v4l2->v4l2_dev; >> + struct v4l2_device *v4l2_dev = dev->v4l2->v4l2_dev; >> dev->ctl_input = index; >> dev->ctl_ainput = INPUT(index)->amux; >> @@ -1565,7 +1573,7 @@ static int vidioc_querystd(struct file *file, >> void *priv, v4l2_std_id *norm) >> { >> struct em28xx *dev = video_drvdata(file); >> - v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, video, querystd, >> norm); >> + v4l2_device_call_all(dev->v4l2->v4l2_dev, 0, video, querystd, norm); >> return 0; >> } >> @@ -1596,7 +1604,7 @@ static int vidioc_s_std(struct file *file, void >> *priv, v4l2_std_id norm) >> &v4l2->hscale, &v4l2->vscale); >> em28xx_resolution_set(dev); >> - v4l2_device_call_all(&v4l2->v4l2_dev, 0, video, s_std, v4l2->norm); >> + v4l2_device_call_all(v4l2->v4l2_dev, 0, video, s_std, v4l2->norm); >> return 0; >> } >> @@ -1616,7 +1624,7 @@ static int vidioc_g_parm(struct file *file, void >> *priv, >> p->parm.capture.readbuffers = EM28XX_MIN_BUF; >> p->parm.capture.capability = V4L2_CAP_TIMEPERFRAME; >> if (dev->is_webcam) { >> - rc = v4l2_device_call_until_err(&v4l2->v4l2_dev, 0, >> + rc = v4l2_device_call_until_err(v4l2->v4l2_dev, 0, >> video, g_frame_interval, &ival); >> if (!rc) >> p->parm.capture.timeperframe = ival.interval; >> @@ -1648,7 +1656,7 @@ static int vidioc_s_parm(struct file *file, void >> *priv, >> memset(&p->parm, 0, sizeof(p->parm)); >> p->parm.capture.readbuffers = EM28XX_MIN_BUF; >> p->parm.capture.capability = V4L2_CAP_TIMEPERFRAME; >> - rc = v4l2_device_call_until_err(&dev->v4l2->v4l2_dev, 0, >> + rc = v4l2_device_call_until_err(dev->v4l2->v4l2_dev, 0, >> video, s_frame_interval, &ival); >> if (!rc) >> p->parm.capture.timeperframe = ival.interval; >> @@ -1675,7 +1683,7 @@ static int vidioc_enum_input(struct file *file, >> void *priv, >> if (INPUT(n)->type == EM28XX_VMUX_TELEVISION) >> i->type = V4L2_INPUT_TYPE_TUNER; >> - i->std = dev->v4l2->vdev.tvnorms; >> + i->std = dev->v4l2->vdev->tvnorms; >> /* webcams do not have the STD API */ >> if (dev->is_webcam) >> i->capabilities = 0; >> @@ -1839,7 +1847,7 @@ static int vidioc_g_tuner(struct file *file, >> void *priv, >> strscpy(t->name, "Tuner", sizeof(t->name)); >> - v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, tuner, g_tuner, t); >> + v4l2_device_call_all(dev->v4l2->v4l2_dev, 0, tuner, g_tuner, t); >> return 0; >> } >> @@ -1851,7 +1859,7 @@ static int vidioc_s_tuner(struct file *file, >> void *priv, >> if (t->index != 0) >> return -EINVAL; >> - v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, tuner, s_tuner, t); >> + v4l2_device_call_all(dev->v4l2->v4l2_dev, 0, tuner, s_tuner, t); >> return 0; >> } >> @@ -1878,8 +1886,8 @@ static int vidioc_s_frequency(struct file *file, >> void *priv, >> if (f->tuner != 0) >> return -EINVAL; >> - v4l2_device_call_all(&v4l2->v4l2_dev, 0, tuner, s_frequency, f); >> - v4l2_device_call_all(&v4l2->v4l2_dev, 0, tuner, g_frequency, >> &new_freq); >> + v4l2_device_call_all(v4l2->v4l2_dev, 0, tuner, s_frequency, f); >> + v4l2_device_call_all(v4l2->v4l2_dev, 0, tuner, g_frequency, >> &new_freq); >> v4l2->frequency = new_freq.frequency; >> return 0; >> @@ -1897,7 +1905,7 @@ static int vidioc_g_chip_info(struct file *file, >> void *priv, >> strscpy(chip->name, "ac97", sizeof(chip->name)); >> else >> strscpy(chip->name, >> - dev->v4l2->v4l2_dev.name, sizeof(chip->name)); >> + dev->v4l2->v4l2_dev->name, sizeof(chip->name)); >> return 0; >> } >> @@ -2095,7 +2103,7 @@ static int radio_g_tuner(struct file *file, void >> *priv, >> strscpy(t->name, "Radio", sizeof(t->name)); >> - v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, tuner, g_tuner, t); >> + v4l2_device_call_all(dev->v4l2->v4l2_dev, 0, tuner, g_tuner, t); >> return 0; >> } >> @@ -2108,7 +2116,7 @@ static int radio_s_tuner(struct file *file, void >> *priv, >> if (t->index != 0) >> return -EINVAL; >> - v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, tuner, s_tuner, t); >> + v4l2_device_call_all(dev->v4l2->v4l2_dev, 0, tuner, s_tuner, t); >> return 0; >> } >> @@ -2160,6 +2168,11 @@ static int em28xx_v4l2_open(struct file *filp) >> if (mutex_lock_interruptible(&dev->lock)) >> return -ERESTARTSYS; >> + if (!dev->v4l2) { >> + mutex_unlock(&dev->lock); >> + return -ENODEV; >> + } >> + >> ret = v4l2_fh_open(filp); >> if (ret) { >> dev_err(&dev->intf->dev, >> @@ -2184,7 +2197,7 @@ static int em28xx_v4l2_open(struct file *filp) >> if (vdev->vfl_type == VFL_TYPE_RADIO) { >> em28xx_videodbg("video_open: setting radio device\n"); >> - v4l2_device_call_all(&v4l2->v4l2_dev, 0, tuner, s_radio); >> + v4l2_device_call_all(v4l2->v4l2_dev, 0, tuner, s_radio); >> } >> kref_get(&dev->ref); >> @@ -2222,7 +2235,7 @@ static int em28xx_v4l2_fini(struct em28xx *dev) >> mutex_lock(&dev->lock); >> - v4l2_device_disconnect(&v4l2->v4l2_dev); >> + v4l2_device_disconnect(v4l2->v4l2_dev); >> em28xx_uninit_usb_xfer(dev, EM28XX_ANALOG_MODE); >> @@ -2238,14 +2251,15 @@ static int em28xx_v4l2_fini(struct em28xx *dev) >> video_device_node_name(&v4l2->vbi_dev)); >> video_unregister_device(&v4l2->vbi_dev); >> } >> - if (video_is_registered(&v4l2->vdev)) { >> + if (video_is_registered(v4l2->vdev)) { >> dev_info(&dev->intf->dev, "V4L2 device %s deregistered\n", >> - video_device_node_name(&v4l2->vdev)); >> - video_unregister_device(&v4l2->vdev); >> + video_device_node_name(v4l2->vdev)); >> + video_unregister_device(v4l2->vdev); >> } >> v4l2_ctrl_handler_free(&v4l2->ctrl_handler); >> - v4l2_device_unregister(&v4l2->v4l2_dev); >> + v4l2_device_unregister(v4l2->v4l2_dev); >> + v4l2_device_put(v4l2->v4l2_dev); >> kref_put(&v4l2->ref, em28xx_free_v4l2); >> @@ -2305,7 +2319,7 @@ static int em28xx_v4l2_close(struct file *filp) >> goto exit; >> /* Save some power by putting tuner to sleep */ >> - v4l2_device_call_all(&v4l2->v4l2_dev, 0, tuner, standby); >> + v4l2_device_call_all(v4l2->v4l2_dev, 0, tuner, standby); >> /* do this before setting alternate! */ >> em28xx_set_mode(dev, EM28XX_SUSPEND); >> @@ -2330,6 +2344,17 @@ static int em28xx_v4l2_close(struct file *filp) >> return 0; >> } >> +void em28xx_vdev_release(struct video_device *vdev) >> +{ >> +#ifdef CONFIG_MEDIA_CONTROLLER >> + int i; >> + >> + for (i = 0; i < vdev->entity.num_pads; i++) >> + kfree(&vdev->entity.pads[i]); >> +#endif >> + kfree(vdev); >> +} >> + >> static const struct v4l2_file_operations em28xx_v4l_fops = { >> .owner = THIS_MODULE, >> .open = em28xx_v4l2_open, >> @@ -2387,7 +2412,7 @@ static const struct v4l2_ioctl_ops >> video_ioctl_ops = { >> static const struct video_device em28xx_video_template = { >> .fops = &em28xx_v4l_fops, >> .ioctl_ops = &video_ioctl_ops, >> - .release = video_device_release_empty, >> + .release = em28xx_vdev_release, >> .tvnorms = V4L2_STD_ALL, >> }; >> @@ -2445,7 +2470,7 @@ static void em28xx_vdev_init(struct em28xx *dev, >> const char *type_name) >> { >> *vfd = *template; >> - vfd->v4l2_dev = &dev->v4l2->v4l2_dev; >> + vfd->v4l2_dev = dev->v4l2->v4l2_dev; >> vfd->lock = &dev->lock; >> if (dev->is_webcam) >> vfd->tvnorms = 0; >> @@ -2459,7 +2484,7 @@ static void em28xx_vdev_init(struct em28xx *dev, >> static void em28xx_tuner_setup(struct em28xx *dev, unsigned short >> tuner_addr) >> { >> struct em28xx_v4l2 *v4l2 = dev->v4l2; >> - struct v4l2_device *v4l2_dev = &v4l2->v4l2_dev; >> + struct v4l2_device *v4l2_dev = v4l2->v4l2_dev; >> struct tuner_setup tun_setup; >> struct v4l2_frequency f; >> @@ -2517,6 +2542,11 @@ static void em28xx_tuner_setup(struct em28xx >> *dev, unsigned short tuner_addr) >> v4l2_device_call_all(v4l2_dev, 0, tuner, s_frequency, &f); >> } >> +void em28xx_v4l2_dev_release(struct v4l2_device *v4l2_dev) >> +{ >> + kfree(v4l2_dev); >> +} >> + >> static int em28xx_v4l2_init(struct em28xx *dev) >> { >> u8 val; >> @@ -2541,26 +2571,35 @@ static int em28xx_v4l2_init(struct em28xx *dev) >> v4l2 = kzalloc(sizeof(*v4l2), GFP_KERNEL); >> if (!v4l2) { >> - mutex_unlock(&dev->lock); >> - return -ENOMEM; >> + ret = -ENOMEM; >> + goto v4l2_error; >> } >> + >> kref_init(&v4l2->ref); >> v4l2->dev = dev; >> dev->v4l2 = v4l2; >> + v4l2->v4l2_dev = kzalloc(sizeof(*v4l2->v4l2_dev), GFP_KERNEL); >> + if (!v4l2->v4l2_dev) { >> + ret = -ENOMEM; >> + goto v4l2_dev_error; >> + } >> + >> + v4l2->v4l2_dev->release = em28xx_v4l2_dev_release; >> + >> #ifdef CONFIG_MEDIA_CONTROLLER >> - v4l2->v4l2_dev.mdev = dev->media_dev; >> + v4l2->v4l2_dev->mdev = dev->media_dev; >> #endif >> - ret = v4l2_device_register(&dev->intf->dev, &v4l2->v4l2_dev); >> + ret = v4l2_device_register(&dev->intf->dev, v4l2->v4l2_dev); >> if (ret < 0) { >> dev_err(&dev->intf->dev, >> "Call to v4l2_device_register() failed!\n"); >> - goto err; >> + goto v4l2_device_register_error; >> } >> hdl = &v4l2->ctrl_handler; >> v4l2_ctrl_handler_init(hdl, 8); >> - v4l2->v4l2_dev.ctrl_handler = hdl; >> + v4l2->v4l2_dev->ctrl_handler = hdl; >> if (dev->is_webcam) >> v4l2->progressive = true; >> @@ -2575,22 +2614,22 @@ static int em28xx_v4l2_init(struct em28xx *dev) >> /* request some modules */ >> if (dev->has_msp34xx) >> - v4l2_i2c_new_subdev(&v4l2->v4l2_dev, >> + v4l2_i2c_new_subdev(v4l2->v4l2_dev, >> &dev->i2c_adap[dev->def_i2c_bus], >> "msp3400", 0, msp3400_addrs); >> if (dev->board.decoder == EM28XX_SAA711X) >> - v4l2_i2c_new_subdev(&v4l2->v4l2_dev, >> + v4l2_i2c_new_subdev(v4l2->v4l2_dev, >> &dev->i2c_adap[dev->def_i2c_bus], >> "saa7115_auto", 0, saa711x_addrs); >> if (dev->board.decoder == EM28XX_TVP5150) >> - v4l2_i2c_new_subdev(&v4l2->v4l2_dev, >> + v4l2_i2c_new_subdev(v4l2->v4l2_dev, >> &dev->i2c_adap[dev->def_i2c_bus], >> "tvp5150", 0, tvp5150_addrs); >> if (dev->board.adecoder == EM28XX_TVAUDIO) >> - v4l2_i2c_new_subdev(&v4l2->v4l2_dev, >> + v4l2_i2c_new_subdev(v4l2->v4l2_dev, >> &dev->i2c_adap[dev->def_i2c_bus], >> "tvaudio", dev->board.tvaudio_addr, NULL); >> @@ -2601,13 +2640,13 @@ static int em28xx_v4l2_init(struct em28xx *dev) >> int has_demod = (dev->board.tda9887_conf & TDA9887_PRESENT); >> if (dev->board.radio.type) >> - v4l2_i2c_new_subdev(&v4l2->v4l2_dev, >> + v4l2_i2c_new_subdev(v4l2->v4l2_dev, >> &dev->i2c_adap[dev->def_i2c_bus], >> "tuner", dev->board.radio_addr, >> NULL); > > Add null check for v4l2_i2c_new_subdev() and error handling. It was okay > check error prior to this change to allocating v4l2_dev. Now this has > to be handled as a error leg. I will add to all bellow. > >> if (has_demod) >> - v4l2_i2c_new_subdev(&v4l2->v4l2_dev, >> + v4l2_i2c_new_subdev(v4l2->v4l2_dev, >> &dev->i2c_adap[dev->def_i2c_bus], >> "tuner", 0, >> v4l2_i2c_tuner_addrs(ADDRS_DEMOD)); > > Same here: > > Add null check for v4l2_i2c_new_subdev() and error handling. It was okay > check error prior to this change to allocating v4l2_dev. Now this has > to be handled as a error leg. > >> @@ -2616,7 +2655,7 @@ static int em28xx_v4l2_init(struct em28xx *dev) >> has_demod ? ADDRS_TV_WITH_DEMOD : ADDRS_TV; >> struct v4l2_subdev *sd; >> - sd = v4l2_i2c_new_subdev(&v4l2->v4l2_dev, >> + sd = v4l2_i2c_new_subdev(v4l2->v4l2_dev, >> &dev->i2c_adap[dev->def_i2c_bus], >> "tuner", 0, >> v4l2_i2c_tuner_addrs(type)); >> @@ -2624,7 +2663,7 @@ static int em28xx_v4l2_init(struct em28xx *dev) >> if (sd) >> tuner_addr = v4l2_i2c_subdev_addr(sd); > > Add null check for v4l2_i2c_new_subdev() and error handling. It was okay > check error prior to this change to allocating v4l2_dev. Now this has > to be handled as a error leg. > >> } else { >> - v4l2_i2c_new_subdev(&v4l2->v4l2_dev, >> + v4l2_i2c_new_subdev(v4l2->v4l2_dev, >> &dev->i2c_adap[dev->def_i2c_bus], >> "tuner", tuner_addr, NULL); > > Add null check for v4l2_i2c_new_subdev() and error handling. It was okay > check error prior to this change to allocating v4l2_dev. Now this has > to be handled as a error leg. > >> } >> @@ -2686,7 +2725,7 @@ static int em28xx_v4l2_init(struct em28xx *dev) >> /* set default norm */ >> v4l2->norm = V4L2_STD_PAL; >> - v4l2_device_call_all(&v4l2->v4l2_dev, 0, video, s_std, v4l2->norm); >> + v4l2_device_call_all(v4l2->v4l2_dev, 0, video, s_std, v4l2->norm); >> v4l2->interlaced_fieldmode = EM28XX_INTERLACED_DEFAULT; >> /* Analog specific initialization */ >> @@ -2756,40 +2795,45 @@ static int em28xx_v4l2_init(struct em28xx *dev) >> goto unregister_dev; >> /* allocate and fill video video_device struct */ >> - em28xx_vdev_init(dev, &v4l2->vdev, &em28xx_video_template, "video"); >> + v4l2->vdev = kzalloc(sizeof(*v4l2->vdev), GFP_KERNEL); >> + if (!v4l2->vdev) { >> + ret = -ENOMEM; >> + goto unregister_dev; >> + } >> + >> + em28xx_vdev_init(dev, v4l2->vdev, &em28xx_video_template, "video"); >> mutex_init(&v4l2->vb_queue_lock); >> mutex_init(&v4l2->vb_vbi_queue_lock); >> - v4l2->vdev.queue = &v4l2->vb_vidq; >> - v4l2->vdev.queue->lock = &v4l2->vb_queue_lock; >> - v4l2->vdev.device_caps = V4L2_CAP_READWRITE | >> V4L2_CAP_VIDEO_CAPTURE | >> + v4l2->vdev->queue = &v4l2->vb_vidq; >> + v4l2->vdev->queue->lock = &v4l2->vb_queue_lock; >> + v4l2->vdev->device_caps = V4L2_CAP_READWRITE | >> V4L2_CAP_VIDEO_CAPTURE | >> V4L2_CAP_STREAMING; >> if (dev->int_audio_type != EM28XX_INT_AUDIO_NONE) >> - v4l2->vdev.device_caps |= V4L2_CAP_AUDIO; >> + v4l2->vdev->device_caps |= V4L2_CAP_AUDIO; >> if (dev->tuner_type != TUNER_ABSENT) >> - v4l2->vdev.device_caps |= V4L2_CAP_TUNER; >> - >> + v4l2->vdev->device_caps |= V4L2_CAP_TUNER; >> /* disable inapplicable ioctls */ >> if (dev->is_webcam) { >> - v4l2_disable_ioctl(&v4l2->vdev, VIDIOC_QUERYSTD); >> - v4l2_disable_ioctl(&v4l2->vdev, VIDIOC_G_STD); >> - v4l2_disable_ioctl(&v4l2->vdev, VIDIOC_S_STD); >> + v4l2_disable_ioctl(v4l2->vdev, VIDIOC_QUERYSTD); >> + v4l2_disable_ioctl(v4l2->vdev, VIDIOC_G_STD); >> + v4l2_disable_ioctl(v4l2->vdev, VIDIOC_S_STD); >> } else { >> - v4l2_disable_ioctl(&v4l2->vdev, VIDIOC_S_PARM); >> + v4l2_disable_ioctl(v4l2->vdev, VIDIOC_S_PARM); >> } >> if (dev->tuner_type == TUNER_ABSENT) { >> - v4l2_disable_ioctl(&v4l2->vdev, VIDIOC_G_TUNER); >> - v4l2_disable_ioctl(&v4l2->vdev, VIDIOC_S_TUNER); >> - v4l2_disable_ioctl(&v4l2->vdev, VIDIOC_G_FREQUENCY); >> - v4l2_disable_ioctl(&v4l2->vdev, VIDIOC_S_FREQUENCY); >> + v4l2_disable_ioctl(v4l2->vdev, VIDIOC_G_TUNER); >> + v4l2_disable_ioctl(v4l2->vdev, VIDIOC_S_TUNER); >> + v4l2_disable_ioctl(v4l2->vdev, VIDIOC_G_FREQUENCY); >> + v4l2_disable_ioctl(v4l2->vdev, VIDIOC_S_FREQUENCY); >> } >> if (dev->int_audio_type == EM28XX_INT_AUDIO_NONE) { >> - v4l2_disable_ioctl(&v4l2->vdev, VIDIOC_G_AUDIO); >> - v4l2_disable_ioctl(&v4l2->vdev, VIDIOC_S_AUDIO); >> + v4l2_disable_ioctl(v4l2->vdev, VIDIOC_G_AUDIO); >> + v4l2_disable_ioctl(v4l2->vdev, VIDIOC_S_AUDIO); >> } >> /* register v4l2 video video_device */ >> - ret = video_register_device(&v4l2->vdev, VFL_TYPE_VIDEO, >> + ret = video_register_device(v4l2->vdev, VFL_TYPE_VIDEO, >> video_nr[dev->devno]); >> if (ret) { >> dev_err(&dev->intf->dev, >> @@ -2863,7 +2907,7 @@ static int em28xx_v4l2_init(struct em28xx *dev) >> dev_info(&dev->intf->dev, >> "V4L2 video device registered as %s\n", >> - video_device_node_name(&v4l2->vdev)); >> + video_device_node_name(v4l2->vdev)); >> if (video_is_registered(&v4l2->vbi_dev)) >> dev_info(&dev->intf->dev, >> @@ -2871,7 +2915,7 @@ static int em28xx_v4l2_init(struct em28xx *dev) >> video_device_node_name(&v4l2->vbi_dev)); >> /* Save some power by putting tuner to sleep */ >> - v4l2_device_call_all(&v4l2->v4l2_dev, 0, tuner, standby); >> + v4l2_device_call_all(v4l2->v4l2_dev, 0, tuner, standby); >> /* initialize videobuf2 stuff */ >> em28xx_vb2_setup(dev); >> @@ -2897,18 +2941,22 @@ static int em28xx_v4l2_init(struct em28xx *dev) >> video_device_node_name(&v4l2->vbi_dev)); >> video_unregister_device(&v4l2->vbi_dev); >> } >> - if (video_is_registered(&v4l2->vdev)) { >> + if (video_is_registered(v4l2->vdev)) { >> dev_info(&dev->intf->dev, >> "V4L2 device %s deregistered\n", >> - video_device_node_name(&v4l2->vdev)); >> - video_unregister_device(&v4l2->vdev); >> + video_device_node_name(v4l2->vdev)); >> + video_unregister_device(v4l2->vdev); >> } >> v4l2_ctrl_handler_free(&v4l2->ctrl_handler); >> - v4l2_device_unregister(&v4l2->v4l2_dev); >> -err: >> + v4l2_device_unregister(v4l2->v4l2_dev); >> + >> +v4l2_device_register_error: >> + v4l2_device_put(v4l2->v4l2_dev); >> +v4l2_dev_error: >> dev->v4l2 = NULL; >> kref_put(&v4l2->ref, em28xx_free_v4l2); >> +v4l2_error: >> mutex_unlock(&dev->lock); >> return ret; >> } >> diff --git a/drivers/media/usb/em28xx/em28xx.h >> b/drivers/media/usb/em28xx/em28xx.h >> index 6648e11f1271..dbcc297b5a0d 100644 >> --- a/drivers/media/usb/em28xx/em28xx.h >> +++ b/drivers/media/usb/em28xx/em28xx.h >> @@ -552,10 +552,10 @@ struct em28xx_v4l2 { >> struct kref ref; >> struct em28xx *dev; >> - struct v4l2_device v4l2_dev; >> + struct v4l2_device *v4l2_dev; >> struct v4l2_ctrl_handler ctrl_handler; >> - struct video_device vdev; >> + struct video_device *vdev; >> struct video_device vbi_dev; >> struct video_device radio_dev; >> @@ -601,7 +601,7 @@ struct em28xx_v4l2 { >> unsigned int field_count; >> #ifdef CONFIG_MEDIA_CONTROLLER >> - struct media_pad video_pad, vbi_pad; >> + struct media_pad *video_pad, vbi_pad; >> struct media_entity *decoder; >> #endif >> }; >> > > thanks, > -- Shuah Thanks, --- Igor Matheus Andrade Torrente
diff --git a/drivers/media/usb/em28xx/em28xx-camera.c b/drivers/media/usb/em28xx/em28xx-camera.c index d1e66b503f4d..436c5a8cbbb6 100644 --- a/drivers/media/usb/em28xx/em28xx-camera.c +++ b/drivers/media/usb/em28xx/em28xx-camera.c @@ -340,7 +340,7 @@ int em28xx_init_camera(struct em28xx *dev) v4l2->sensor_xtal = 4300000; pdata.xtal = v4l2->sensor_xtal; if (NULL == - v4l2_i2c_new_subdev_board(&v4l2->v4l2_dev, adap, + v4l2_i2c_new_subdev_board(v4l2->v4l2_dev, adap, &mt9v011_info, NULL)) return -ENODEV; v4l2->vinmode = EM28XX_VINMODE_RGB8_GRBG; @@ -394,7 +394,7 @@ int em28xx_init_camera(struct em28xx *dev) v4l2->sensor_yres = 480; subdev = - v4l2_i2c_new_subdev_board(&v4l2->v4l2_dev, adap, + v4l2_i2c_new_subdev_board(v4l2->v4l2_dev, adap, &ov2640_info, NULL); if (!subdev) return -ENODEV; diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c index 6b84c3413e83..e1febb2bf06b 100644 --- a/drivers/media/usb/em28xx/em28xx-video.c +++ b/drivers/media/usb/em28xx/em28xx-video.c @@ -184,7 +184,7 @@ static int em28xx_vbi_supported(struct em28xx *dev) */ static void em28xx_wake_i2c(struct em28xx *dev) { - struct v4l2_device *v4l2_dev = &dev->v4l2->v4l2_dev; + struct v4l2_device *v4l2_dev = dev->v4l2->v4l2_dev; v4l2_device_call_all(v4l2_dev, 0, core, reset, 0); v4l2_device_call_all(v4l2_dev, 0, video, s_routing, @@ -974,9 +974,17 @@ static void em28xx_v4l2_create_entities(struct em28xx *dev) struct em28xx_v4l2 *v4l2 = dev->v4l2; int ret, i; + v4l2->video_pad = kzalloc(sizeof(*v4l2->video_pad), GFP_KERNEL); + if (!v4l2->video_pad) { + dev_err(&dev->intf->dev, + "failed to allocate video pad memory!\n"); + v4l2->vdev->entity.num_pads = 0; + return; + } + /* Initialize Video, VBI and Radio pads */ - v4l2->video_pad.flags = MEDIA_PAD_FL_SINK; - ret = media_entity_pads_init(&v4l2->vdev.entity, 1, &v4l2->video_pad); + v4l2->video_pad->flags = MEDIA_PAD_FL_SINK; + ret = media_entity_pads_init(&v4l2->vdev->entity, 1, v4l2->video_pad); if (ret < 0) dev_err(&dev->intf->dev, "failed to initialize video media entity!\n"); @@ -1132,11 +1140,11 @@ int em28xx_start_analog_streaming(struct vb2_queue *vq, unsigned int count) f.type = V4L2_TUNER_RADIO; else f.type = V4L2_TUNER_ANALOG_TV; - v4l2_device_call_all(&v4l2->v4l2_dev, + v4l2_device_call_all(v4l2->v4l2_dev, 0, tuner, s_frequency, &f); /* Enable video stream at TV decoder */ - v4l2_device_call_all(&v4l2->v4l2_dev, 0, video, s_stream, 1); + v4l2_device_call_all(v4l2->v4l2_dev, 0, video, s_stream, 1); } v4l2->streaming_users++; @@ -1157,7 +1165,7 @@ static void em28xx_stop_streaming(struct vb2_queue *vq) if (v4l2->streaming_users-- == 1) { /* Disable video stream at TV decoder */ - v4l2_device_call_all(&v4l2->v4l2_dev, 0, video, s_stream, 0); + v4l2_device_call_all(v4l2->v4l2_dev, 0, video, s_stream, 0); /* Last active user, so shutdown all the URBS */ em28xx_uninit_usb_xfer(dev, EM28XX_ANALOG_MODE); @@ -1192,7 +1200,7 @@ void em28xx_stop_vbi_streaming(struct vb2_queue *vq) if (v4l2->streaming_users-- == 1) { /* Disable video stream at TV decoder */ - v4l2_device_call_all(&v4l2->v4l2_dev, 0, video, s_stream, 0); + v4l2_device_call_all(v4l2->v4l2_dev, 0, video, s_stream, 0); /* Last active user, so shutdown all the URBS */ em28xx_uninit_usb_xfer(dev, EM28XX_ANALOG_MODE); @@ -1286,7 +1294,7 @@ static int em28xx_vb2_setup(struct em28xx *dev) static void video_mux(struct em28xx *dev, int index) { - struct v4l2_device *v4l2_dev = &dev->v4l2->v4l2_dev; + struct v4l2_device *v4l2_dev = dev->v4l2->v4l2_dev; dev->ctl_input = index; dev->ctl_ainput = INPUT(index)->amux; @@ -1565,7 +1573,7 @@ static int vidioc_querystd(struct file *file, void *priv, v4l2_std_id *norm) { struct em28xx *dev = video_drvdata(file); - v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, video, querystd, norm); + v4l2_device_call_all(dev->v4l2->v4l2_dev, 0, video, querystd, norm); return 0; } @@ -1596,7 +1604,7 @@ static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id norm) &v4l2->hscale, &v4l2->vscale); em28xx_resolution_set(dev); - v4l2_device_call_all(&v4l2->v4l2_dev, 0, video, s_std, v4l2->norm); + v4l2_device_call_all(v4l2->v4l2_dev, 0, video, s_std, v4l2->norm); return 0; } @@ -1616,7 +1624,7 @@ static int vidioc_g_parm(struct file *file, void *priv, p->parm.capture.readbuffers = EM28XX_MIN_BUF; p->parm.capture.capability = V4L2_CAP_TIMEPERFRAME; if (dev->is_webcam) { - rc = v4l2_device_call_until_err(&v4l2->v4l2_dev, 0, + rc = v4l2_device_call_until_err(v4l2->v4l2_dev, 0, video, g_frame_interval, &ival); if (!rc) p->parm.capture.timeperframe = ival.interval; @@ -1648,7 +1656,7 @@ static int vidioc_s_parm(struct file *file, void *priv, memset(&p->parm, 0, sizeof(p->parm)); p->parm.capture.readbuffers = EM28XX_MIN_BUF; p->parm.capture.capability = V4L2_CAP_TIMEPERFRAME; - rc = v4l2_device_call_until_err(&dev->v4l2->v4l2_dev, 0, + rc = v4l2_device_call_until_err(dev->v4l2->v4l2_dev, 0, video, s_frame_interval, &ival); if (!rc) p->parm.capture.timeperframe = ival.interval; @@ -1675,7 +1683,7 @@ static int vidioc_enum_input(struct file *file, void *priv, if (INPUT(n)->type == EM28XX_VMUX_TELEVISION) i->type = V4L2_INPUT_TYPE_TUNER; - i->std = dev->v4l2->vdev.tvnorms; + i->std = dev->v4l2->vdev->tvnorms; /* webcams do not have the STD API */ if (dev->is_webcam) i->capabilities = 0; @@ -1839,7 +1847,7 @@ static int vidioc_g_tuner(struct file *file, void *priv, strscpy(t->name, "Tuner", sizeof(t->name)); - v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, tuner, g_tuner, t); + v4l2_device_call_all(dev->v4l2->v4l2_dev, 0, tuner, g_tuner, t); return 0; } @@ -1851,7 +1859,7 @@ static int vidioc_s_tuner(struct file *file, void *priv, if (t->index != 0) return -EINVAL; - v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, tuner, s_tuner, t); + v4l2_device_call_all(dev->v4l2->v4l2_dev, 0, tuner, s_tuner, t); return 0; } @@ -1878,8 +1886,8 @@ static int vidioc_s_frequency(struct file *file, void *priv, if (f->tuner != 0) return -EINVAL; - v4l2_device_call_all(&v4l2->v4l2_dev, 0, tuner, s_frequency, f); - v4l2_device_call_all(&v4l2->v4l2_dev, 0, tuner, g_frequency, &new_freq); + v4l2_device_call_all(v4l2->v4l2_dev, 0, tuner, s_frequency, f); + v4l2_device_call_all(v4l2->v4l2_dev, 0, tuner, g_frequency, &new_freq); v4l2->frequency = new_freq.frequency; return 0; @@ -1897,7 +1905,7 @@ static int vidioc_g_chip_info(struct file *file, void *priv, strscpy(chip->name, "ac97", sizeof(chip->name)); else strscpy(chip->name, - dev->v4l2->v4l2_dev.name, sizeof(chip->name)); + dev->v4l2->v4l2_dev->name, sizeof(chip->name)); return 0; } @@ -2095,7 +2103,7 @@ static int radio_g_tuner(struct file *file, void *priv, strscpy(t->name, "Radio", sizeof(t->name)); - v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, tuner, g_tuner, t); + v4l2_device_call_all(dev->v4l2->v4l2_dev, 0, tuner, g_tuner, t); return 0; } @@ -2108,7 +2116,7 @@ static int radio_s_tuner(struct file *file, void *priv, if (t->index != 0) return -EINVAL; - v4l2_device_call_all(&dev->v4l2->v4l2_dev, 0, tuner, s_tuner, t); + v4l2_device_call_all(dev->v4l2->v4l2_dev, 0, tuner, s_tuner, t); return 0; } @@ -2160,6 +2168,11 @@ static int em28xx_v4l2_open(struct file *filp) if (mutex_lock_interruptible(&dev->lock)) return -ERESTARTSYS; + if (!dev->v4l2) { + mutex_unlock(&dev->lock); + return -ENODEV; + } + ret = v4l2_fh_open(filp); if (ret) { dev_err(&dev->intf->dev, @@ -2184,7 +2197,7 @@ static int em28xx_v4l2_open(struct file *filp) if (vdev->vfl_type == VFL_TYPE_RADIO) { em28xx_videodbg("video_open: setting radio device\n"); - v4l2_device_call_all(&v4l2->v4l2_dev, 0, tuner, s_radio); + v4l2_device_call_all(v4l2->v4l2_dev, 0, tuner, s_radio); } kref_get(&dev->ref); @@ -2222,7 +2235,7 @@ static int em28xx_v4l2_fini(struct em28xx *dev) mutex_lock(&dev->lock); - v4l2_device_disconnect(&v4l2->v4l2_dev); + v4l2_device_disconnect(v4l2->v4l2_dev); em28xx_uninit_usb_xfer(dev, EM28XX_ANALOG_MODE); @@ -2238,14 +2251,15 @@ static int em28xx_v4l2_fini(struct em28xx *dev) video_device_node_name(&v4l2->vbi_dev)); video_unregister_device(&v4l2->vbi_dev); } - if (video_is_registered(&v4l2->vdev)) { + if (video_is_registered(v4l2->vdev)) { dev_info(&dev->intf->dev, "V4L2 device %s deregistered\n", - video_device_node_name(&v4l2->vdev)); - video_unregister_device(&v4l2->vdev); + video_device_node_name(v4l2->vdev)); + video_unregister_device(v4l2->vdev); } v4l2_ctrl_handler_free(&v4l2->ctrl_handler); - v4l2_device_unregister(&v4l2->v4l2_dev); + v4l2_device_unregister(v4l2->v4l2_dev); + v4l2_device_put(v4l2->v4l2_dev); kref_put(&v4l2->ref, em28xx_free_v4l2); @@ -2305,7 +2319,7 @@ static int em28xx_v4l2_close(struct file *filp) goto exit; /* Save some power by putting tuner to sleep */ - v4l2_device_call_all(&v4l2->v4l2_dev, 0, tuner, standby); + v4l2_device_call_all(v4l2->v4l2_dev, 0, tuner, standby); /* do this before setting alternate! */ em28xx_set_mode(dev, EM28XX_SUSPEND); @@ -2330,6 +2344,17 @@ static int em28xx_v4l2_close(struct file *filp) return 0; } +void em28xx_vdev_release(struct video_device *vdev) +{ +#ifdef CONFIG_MEDIA_CONTROLLER + int i; + + for (i = 0; i < vdev->entity.num_pads; i++) + kfree(&vdev->entity.pads[i]); +#endif + kfree(vdev); +} + static const struct v4l2_file_operations em28xx_v4l_fops = { .owner = THIS_MODULE, .open = em28xx_v4l2_open, @@ -2387,7 +2412,7 @@ static const struct v4l2_ioctl_ops video_ioctl_ops = { static const struct video_device em28xx_video_template = { .fops = &em28xx_v4l_fops, .ioctl_ops = &video_ioctl_ops, - .release = video_device_release_empty, + .release = em28xx_vdev_release, .tvnorms = V4L2_STD_ALL, }; @@ -2445,7 +2470,7 @@ static void em28xx_vdev_init(struct em28xx *dev, const char *type_name) { *vfd = *template; - vfd->v4l2_dev = &dev->v4l2->v4l2_dev; + vfd->v4l2_dev = dev->v4l2->v4l2_dev; vfd->lock = &dev->lock; if (dev->is_webcam) vfd->tvnorms = 0; @@ -2459,7 +2484,7 @@ static void em28xx_vdev_init(struct em28xx *dev, static void em28xx_tuner_setup(struct em28xx *dev, unsigned short tuner_addr) { struct em28xx_v4l2 *v4l2 = dev->v4l2; - struct v4l2_device *v4l2_dev = &v4l2->v4l2_dev; + struct v4l2_device *v4l2_dev = v4l2->v4l2_dev; struct tuner_setup tun_setup; struct v4l2_frequency f; @@ -2517,6 +2542,11 @@ static void em28xx_tuner_setup(struct em28xx *dev, unsigned short tuner_addr) v4l2_device_call_all(v4l2_dev, 0, tuner, s_frequency, &f); } +void em28xx_v4l2_dev_release(struct v4l2_device *v4l2_dev) +{ + kfree(v4l2_dev); +} + static int em28xx_v4l2_init(struct em28xx *dev) { u8 val; @@ -2541,26 +2571,35 @@ static int em28xx_v4l2_init(struct em28xx *dev) v4l2 = kzalloc(sizeof(*v4l2), GFP_KERNEL); if (!v4l2) { - mutex_unlock(&dev->lock); - return -ENOMEM; + ret = -ENOMEM; + goto v4l2_error; } + kref_init(&v4l2->ref); v4l2->dev = dev; dev->v4l2 = v4l2; + v4l2->v4l2_dev = kzalloc(sizeof(*v4l2->v4l2_dev), GFP_KERNEL); + if (!v4l2->v4l2_dev) { + ret = -ENOMEM; + goto v4l2_dev_error; + } + + v4l2->v4l2_dev->release = em28xx_v4l2_dev_release; + #ifdef CONFIG_MEDIA_CONTROLLER - v4l2->v4l2_dev.mdev = dev->media_dev; + v4l2->v4l2_dev->mdev = dev->media_dev; #endif - ret = v4l2_device_register(&dev->intf->dev, &v4l2->v4l2_dev); + ret = v4l2_device_register(&dev->intf->dev, v4l2->v4l2_dev); if (ret < 0) { dev_err(&dev->intf->dev, "Call to v4l2_device_register() failed!\n"); - goto err; + goto v4l2_device_register_error; } hdl = &v4l2->ctrl_handler; v4l2_ctrl_handler_init(hdl, 8); - v4l2->v4l2_dev.ctrl_handler = hdl; + v4l2->v4l2_dev->ctrl_handler = hdl; if (dev->is_webcam) v4l2->progressive = true; @@ -2575,22 +2614,22 @@ static int em28xx_v4l2_init(struct em28xx *dev) /* request some modules */ if (dev->has_msp34xx) - v4l2_i2c_new_subdev(&v4l2->v4l2_dev, + v4l2_i2c_new_subdev(v4l2->v4l2_dev, &dev->i2c_adap[dev->def_i2c_bus], "msp3400", 0, msp3400_addrs); if (dev->board.decoder == EM28XX_SAA711X) - v4l2_i2c_new_subdev(&v4l2->v4l2_dev, + v4l2_i2c_new_subdev(v4l2->v4l2_dev, &dev->i2c_adap[dev->def_i2c_bus], "saa7115_auto", 0, saa711x_addrs); if (dev->board.decoder == EM28XX_TVP5150) - v4l2_i2c_new_subdev(&v4l2->v4l2_dev, + v4l2_i2c_new_subdev(v4l2->v4l2_dev, &dev->i2c_adap[dev->def_i2c_bus], "tvp5150", 0, tvp5150_addrs); if (dev->board.adecoder == EM28XX_TVAUDIO) - v4l2_i2c_new_subdev(&v4l2->v4l2_dev, + v4l2_i2c_new_subdev(v4l2->v4l2_dev, &dev->i2c_adap[dev->def_i2c_bus], "tvaudio", dev->board.tvaudio_addr, NULL); @@ -2601,13 +2640,13 @@ static int em28xx_v4l2_init(struct em28xx *dev) int has_demod = (dev->board.tda9887_conf & TDA9887_PRESENT); if (dev->board.radio.type) - v4l2_i2c_new_subdev(&v4l2->v4l2_dev, + v4l2_i2c_new_subdev(v4l2->v4l2_dev, &dev->i2c_adap[dev->def_i2c_bus], "tuner", dev->board.radio_addr, NULL); if (has_demod) - v4l2_i2c_new_subdev(&v4l2->v4l2_dev, + v4l2_i2c_new_subdev(v4l2->v4l2_dev, &dev->i2c_adap[dev->def_i2c_bus], "tuner", 0, v4l2_i2c_tuner_addrs(ADDRS_DEMOD)); @@ -2616,7 +2655,7 @@ static int em28xx_v4l2_init(struct em28xx *dev) has_demod ? ADDRS_TV_WITH_DEMOD : ADDRS_TV; struct v4l2_subdev *sd; - sd = v4l2_i2c_new_subdev(&v4l2->v4l2_dev, + sd = v4l2_i2c_new_subdev(v4l2->v4l2_dev, &dev->i2c_adap[dev->def_i2c_bus], "tuner", 0, v4l2_i2c_tuner_addrs(type)); @@ -2624,7 +2663,7 @@ static int em28xx_v4l2_init(struct em28xx *dev) if (sd) tuner_addr = v4l2_i2c_subdev_addr(sd); } else { - v4l2_i2c_new_subdev(&v4l2->v4l2_dev, + v4l2_i2c_new_subdev(v4l2->v4l2_dev, &dev->i2c_adap[dev->def_i2c_bus], "tuner", tuner_addr, NULL); } @@ -2686,7 +2725,7 @@ static int em28xx_v4l2_init(struct em28xx *dev) /* set default norm */ v4l2->norm = V4L2_STD_PAL; - v4l2_device_call_all(&v4l2->v4l2_dev, 0, video, s_std, v4l2->norm); + v4l2_device_call_all(v4l2->v4l2_dev, 0, video, s_std, v4l2->norm); v4l2->interlaced_fieldmode = EM28XX_INTERLACED_DEFAULT; /* Analog specific initialization */ @@ -2756,40 +2795,45 @@ static int em28xx_v4l2_init(struct em28xx *dev) goto unregister_dev; /* allocate and fill video video_device struct */ - em28xx_vdev_init(dev, &v4l2->vdev, &em28xx_video_template, "video"); + v4l2->vdev = kzalloc(sizeof(*v4l2->vdev), GFP_KERNEL); + if (!v4l2->vdev) { + ret = -ENOMEM; + goto unregister_dev; + } + + em28xx_vdev_init(dev, v4l2->vdev, &em28xx_video_template, "video"); mutex_init(&v4l2->vb_queue_lock); mutex_init(&v4l2->vb_vbi_queue_lock); - v4l2->vdev.queue = &v4l2->vb_vidq; - v4l2->vdev.queue->lock = &v4l2->vb_queue_lock; - v4l2->vdev.device_caps = V4L2_CAP_READWRITE | V4L2_CAP_VIDEO_CAPTURE | + v4l2->vdev->queue = &v4l2->vb_vidq; + v4l2->vdev->queue->lock = &v4l2->vb_queue_lock; + v4l2->vdev->device_caps = V4L2_CAP_READWRITE | V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING; if (dev->int_audio_type != EM28XX_INT_AUDIO_NONE) - v4l2->vdev.device_caps |= V4L2_CAP_AUDIO; + v4l2->vdev->device_caps |= V4L2_CAP_AUDIO; if (dev->tuner_type != TUNER_ABSENT) - v4l2->vdev.device_caps |= V4L2_CAP_TUNER; - + v4l2->vdev->device_caps |= V4L2_CAP_TUNER; /* disable inapplicable ioctls */ if (dev->is_webcam) { - v4l2_disable_ioctl(&v4l2->vdev, VIDIOC_QUERYSTD); - v4l2_disable_ioctl(&v4l2->vdev, VIDIOC_G_STD); - v4l2_disable_ioctl(&v4l2->vdev, VIDIOC_S_STD); + v4l2_disable_ioctl(v4l2->vdev, VIDIOC_QUERYSTD); + v4l2_disable_ioctl(v4l2->vdev, VIDIOC_G_STD); + v4l2_disable_ioctl(v4l2->vdev, VIDIOC_S_STD); } else { - v4l2_disable_ioctl(&v4l2->vdev, VIDIOC_S_PARM); + v4l2_disable_ioctl(v4l2->vdev, VIDIOC_S_PARM); } if (dev->tuner_type == TUNER_ABSENT) { - v4l2_disable_ioctl(&v4l2->vdev, VIDIOC_G_TUNER); - v4l2_disable_ioctl(&v4l2->vdev, VIDIOC_S_TUNER); - v4l2_disable_ioctl(&v4l2->vdev, VIDIOC_G_FREQUENCY); - v4l2_disable_ioctl(&v4l2->vdev, VIDIOC_S_FREQUENCY); + v4l2_disable_ioctl(v4l2->vdev, VIDIOC_G_TUNER); + v4l2_disable_ioctl(v4l2->vdev, VIDIOC_S_TUNER); + v4l2_disable_ioctl(v4l2->vdev, VIDIOC_G_FREQUENCY); + v4l2_disable_ioctl(v4l2->vdev, VIDIOC_S_FREQUENCY); } if (dev->int_audio_type == EM28XX_INT_AUDIO_NONE) { - v4l2_disable_ioctl(&v4l2->vdev, VIDIOC_G_AUDIO); - v4l2_disable_ioctl(&v4l2->vdev, VIDIOC_S_AUDIO); + v4l2_disable_ioctl(v4l2->vdev, VIDIOC_G_AUDIO); + v4l2_disable_ioctl(v4l2->vdev, VIDIOC_S_AUDIO); } /* register v4l2 video video_device */ - ret = video_register_device(&v4l2->vdev, VFL_TYPE_VIDEO, + ret = video_register_device(v4l2->vdev, VFL_TYPE_VIDEO, video_nr[dev->devno]); if (ret) { dev_err(&dev->intf->dev, @@ -2863,7 +2907,7 @@ static int em28xx_v4l2_init(struct em28xx *dev) dev_info(&dev->intf->dev, "V4L2 video device registered as %s\n", - video_device_node_name(&v4l2->vdev)); + video_device_node_name(v4l2->vdev)); if (video_is_registered(&v4l2->vbi_dev)) dev_info(&dev->intf->dev, @@ -2871,7 +2915,7 @@ static int em28xx_v4l2_init(struct em28xx *dev) video_device_node_name(&v4l2->vbi_dev)); /* Save some power by putting tuner to sleep */ - v4l2_device_call_all(&v4l2->v4l2_dev, 0, tuner, standby); + v4l2_device_call_all(v4l2->v4l2_dev, 0, tuner, standby); /* initialize videobuf2 stuff */ em28xx_vb2_setup(dev); @@ -2897,18 +2941,22 @@ static int em28xx_v4l2_init(struct em28xx *dev) video_device_node_name(&v4l2->vbi_dev)); video_unregister_device(&v4l2->vbi_dev); } - if (video_is_registered(&v4l2->vdev)) { + if (video_is_registered(v4l2->vdev)) { dev_info(&dev->intf->dev, "V4L2 device %s deregistered\n", - video_device_node_name(&v4l2->vdev)); - video_unregister_device(&v4l2->vdev); + video_device_node_name(v4l2->vdev)); + video_unregister_device(v4l2->vdev); } v4l2_ctrl_handler_free(&v4l2->ctrl_handler); - v4l2_device_unregister(&v4l2->v4l2_dev); -err: + v4l2_device_unregister(v4l2->v4l2_dev); + +v4l2_device_register_error: + v4l2_device_put(v4l2->v4l2_dev); +v4l2_dev_error: dev->v4l2 = NULL; kref_put(&v4l2->ref, em28xx_free_v4l2); +v4l2_error: mutex_unlock(&dev->lock); return ret; } diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h index 6648e11f1271..dbcc297b5a0d 100644 --- a/drivers/media/usb/em28xx/em28xx.h +++ b/drivers/media/usb/em28xx/em28xx.h @@ -552,10 +552,10 @@ struct em28xx_v4l2 { struct kref ref; struct em28xx *dev; - struct v4l2_device v4l2_dev; + struct v4l2_device *v4l2_dev; struct v4l2_ctrl_handler ctrl_handler; - struct video_device vdev; + struct video_device *vdev; struct video_device vbi_dev; struct video_device radio_dev; @@ -601,7 +601,7 @@ struct em28xx_v4l2 { unsigned int field_count; #ifdef CONFIG_MEDIA_CONTROLLER - struct media_pad video_pad, vbi_pad; + struct media_pad *video_pad, vbi_pad; struct media_entity *decoder; #endif };
Fixes a race condition - for lack of a more precise term - between em28xx_v4l2_open and em28xx_v4l2_init, by detaching the v4l2_dev, media_pad and vdev structs from the em28xx_v4l2, and managing the lifetime of those objects more dynamicaly. The race happens when a thread[1] - containing the em28xx_v4l2_init() code - calls the v4l2_mc_create_media_graph(), and it return a error, if a thread[2] - running v4l2_open() - pass the verification point and reaches the em28xx_v4l2_open() before the thread[1] finishes the v4l2 subsystem deregistration, thread[1] will free all resources before the em28xx_v4l2_open() can process their things, because the em28xx_v4l2_init() has the dev->lock. And all this lead the thread[2] to cause a user-after-free. Reported-and-tested-by: syzbot+b2391895514ed9ef4a8e@syzkaller.appspotmail.com Signed-off-by: Igor Matheus Andrade Torrente <igormtorrente@gmail.com> --- drivers/media/usb/em28xx/em28xx-camera.c | 4 +- drivers/media/usb/em28xx/em28xx-video.c | 188 ++++++++++++++--------- drivers/media/usb/em28xx/em28xx.h | 6 +- 3 files changed, 123 insertions(+), 75 deletions(-)