[ 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
