[ Hi Hans,

  This might not really be your bug, but I just respect you a lot and
  so I always come to you with questions and for advice.  -dan ]

Hello Hans Verkuil,

The patch 6680427791c9: "[media] mxb: fix audio handling" from Apr
30, 2012, leads to the following static checker warning:

        drivers/media/pci/saa7146/mxb.c:196 tea6420_route()
        warn: uncapped user index 'TEA6420_cd[idx]'

drivers/media/pci/saa7146/mxb.c
    194 static inline void tea6420_route(struct mxb *mxb, int idx)
    195 {
--> 196         v4l2_subdev_call(mxb->tea6420_1, audio, s_routing,
    197                 TEA6420_cd[idx][0].input, TEA6420_cd[idx][0].output, 0);
                                   ^^^
Index overflow.  The TEA6420_cd[] array has MXB_AUDIOS + 1 (which is 7
altogether) elements.

    198         v4l2_subdev_call(mxb->tea6420_2, audio, s_routing,
    199                 TEA6420_cd[idx][1].input, TEA6420_cd[idx][1].output, 0);
    200         v4l2_subdev_call(mxb->tea6420_1, audio, s_routing,
    201                 TEA6420_line[idx][0].input, 
TEA6420_line[idx][0].output, 0);
    202         v4l2_subdev_call(mxb->tea6420_2, audio, s_routing,
    203                 TEA6420_line[idx][1].input, 
TEA6420_line[idx][1].output, 0);
    204 }

[ snip ]

    650  static int vidioc_s_audio(struct file *file, void *fh, const struct 
v4l2_audio *a)
    651  {
    652          struct saa7146_dev *dev = ((struct saa7146_fh *)fh)->dev;
    653          struct mxb *mxb = (struct mxb *)dev->ext_priv;
    654  
    655          DEB_D("VIDIOC_S_AUDIO %d\n", a->index);
    656          if (mxb_inputs[mxb->cur_input].audioset & (1 << a->index)) {

This a->index comes from the ioctl and it's a u32 so the shift can wrap.
The .audioset variable is always 0x3f.  In other words BIT(6) is the
highest valid bit so we could add a check:

                if (a->index > MXB_AUDIOS)
                        return;

    657                  if (mxb->cur_audinput != a->index) {
    658                          mxb->cur_audinput = a->index;
                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Now here's the complication.  We also use a->index as an index into the
mxb_inputs[] array which only has MXB_INPUTS (4) elements, so just
adding the limit would still lead to a different array out of bounds
later...

    659                          tea6420_route(mxb, a->index);
    660                          if (mxb->cur_audinput == 0)
    661                                  mxb_update_audmode(mxb);
    662                  }
    663                  return 0;
    664          }
    665          return -EINVAL;
    666  }

regards,
dan carpenter

Reply via email to