| Message ID | 20130109073632.GD2454@elgon.mountain (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Em Wed, 9 Jan 2013 10:36:32 +0300 Dan Carpenter <dan.carpenter@oracle.com> escreveu: > This is a static checker fix. In the ttusb_process_muxpack() we do: > > cc = (muxpack[len - 4] << 8) | muxpack[len - 3]; > > That means if we pass a number less than 4 then we will either trigger a > checksum error message or read before the start of the array. > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > I can't test this. > > This patch doesn't introduce any bugs, but I'm not positive this is the > right thing to do. Perhaps it's better to print an error message? I don't have any ttusb device either, but i suspect that printing an error message inside ttusb_process_muxpack() would be better. From what I understood, this code gets the URB data and groups it into one TS packet (188 bytes, typically). Then, it calls ttusb_process_muxpack() in order to handle it. So, the normal condition would be to always receive 188 bytes here (usual TS packet size), except if there's something wrong with the URB transfer. It seems, however, that there are other issues at the logic at ttusb_process_muxpack(). For example, from this code snippet: for (i = 0; i < len; i += 2) csum ^= le16_to_cpup((__le16 *) (muxpack + i)); an odd value for len also seems to cause troubles at this logic. so, IMHO, the better would be to print a warning if the value is odd or smaller than 4, and discard it. > > diff --git a/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c b/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c > index 5b682cc..99a2fd1 100644 > --- a/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c > +++ b/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c > @@ -709,7 +709,7 @@ static void ttusb_process_frame(struct ttusb *ttusb, u8 * data, int len) > * if length is valid and we reached the end: > * goto next muxpack > */ > - if ((ttusb->muxpack_ptr >= 2) && > + if ((ttusb->muxpack_ptr >= 4) && > (ttusb->muxpack_ptr == > ttusb->muxpack_len)) { > ttusb_process_muxpack(ttusb, > -- > To unsubscribe from this list: send the line "unsubscribe linux-media" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c b/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c index 5b682cc..99a2fd1 100644 --- a/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c +++ b/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c @@ -709,7 +709,7 @@ static void ttusb_process_frame(struct ttusb *ttusb, u8 * data, int len) * if length is valid and we reached the end: * goto next muxpack */ - if ((ttusb->muxpack_ptr >= 2) && + if ((ttusb->muxpack_ptr >= 4) && (ttusb->muxpack_ptr == ttusb->muxpack_len)) { ttusb_process_muxpack(ttusb,
This is a static checker fix. In the ttusb_process_muxpack() we do: cc = (muxpack[len - 4] << 8) | muxpack[len - 3]; That means if we pass a number less than 4 then we will either trigger a checksum error message or read before the start of the array. Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- I can't test this. This patch doesn't introduce any bugs, but I'm not positive this is the right thing to do. Perhaps it's better to print an error message? -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html