[bug report] [media] davinci: vpfe: add v4l2 video driver support

[bug report] [media] davinci: vpfe: add v4l2 video driver support

[Dan Carpenter]

[ This is really old, but it looks like a potentially serious security
  bug so we probably want to fix it.  -dan ]

Hello Manjunath Hadli,

The patch 622897da67b3: "[media] davinci: vpfe: add v4l2 video driver
support" from Nov 28, 2012, leads to the following static checker
warning:

	drivers/staging/media/davinci_vpfe/vpfe_video.c:871 vpfe_s_input()
	warn: uncapped user index 'sdinfo->routes[index]'

drivers/staging/media/davinci_vpfe/vpfe_video.c
   821  /*
   822   * vpfe_s_input() - set input which is pointed by input index
   823   * @file: file pointer
   824   * @priv: void pointer
   825   * @index: pointer to unsigned int
   826   *
   827   * set input on external subdev
   828   *
   829   * Return 0 on success, error code otherwise
   830   */
   831  static int vpfe_s_input(struct file *file, void *priv, unsigned int index)
                                                               ^^^^^^^^^^^^^^^^^^
index comes from __video_do_ioctl() -> v4l_s_input() -> vpfe_s_input().
It hasn't been checked.

   832  {
   833          struct vpfe_video_device *video = video_drvdata(file);
   834          struct vpfe_device *vpfe_dev = video->vpfe_dev;
   835          struct vpfe_ext_subdev_info *sdinfo;
   836          struct vpfe_route *route;
   837          struct v4l2_input *inps;
   838          u32 output;
   839          u32 input;
   840          int ret;
   841          int i;
   842  
   843          v4l2_dbg(1, debug, &vpfe_dev->v4l2_dev, "vpfe_s_input\n");
   844  
   845          ret = mutex_lock_interruptible(&video->lock);
   846          if (ret)
   847                  return ret;
   848          /*
   849           * If streaming is started return device busy
   850           * error
   851           */
   852          if (video->started) {
   853                  v4l2_err(&vpfe_dev->v4l2_dev, "Streaming is on\n");
   854                  ret = -EBUSY;
   855                  goto unlock_out;
   856          }
   857  
   858          sdinfo = video->current_ext_subdev;
   859          if (!sdinfo->registered) {
   860                  ret = -EINVAL;
   861                  goto unlock_out;
   862          }
   863          if (vpfe_dev->cfg->setup_input &&
   864                  vpfe_dev->cfg->setup_input(sdinfo->grp_id) < 0) {
   865                  ret = -EFAULT;
   866                  v4l2_dbg(1, debug, &vpfe_dev->v4l2_dev,
   867                            "couldn't setup input for %s\n",
   868                            sdinfo->module_name);
   869                  goto unlock_out;
   870          }
   871          route = &sdinfo->routes[index];

We're potentially reading out of bounds here.  The problem is that we
don't store the size of the ->routes[] array anywhere (it has a sentinal
at the end) so I'm not sure what to check against.

Please CC me on the fix.

   872          if (route && sdinfo->can_route) {
   873                  input = route->input;
   874                  output = route->output;
   875                  ret = v4l2_device_call_until_err(&vpfe_dev->v4l2_dev,
   876                                                   sdinfo->grp_id, video,
   877                                                   s_routing, input, output, 0);
   878                  if (ret) {
   879                          v4l2_dbg(1, debug, &vpfe_dev->v4l2_dev,
   880                                  "s_input:error in setting input in decoder\n");
   881                          ret = -EINVAL;
   882                          goto unlock_out;
   883                  }
   884          }
   885          /* set standards set by subdev in video device */
   886          for (i = 0; i < sdinfo->num_inputs; i++) {
   887                  inps = &sdinfo->inputs[i];
   888                  video->video_dev.tvnorms |= inps->std;
   889          }
   890          video->current_input = index;
   891  unlock_out:
   892          mutex_unlock(&video->lock);
   893          return ret;
   894  }

regards,
dan carpenter