[bug report] ti-st: potential overflow calling st_send_frame()

[Savoy, Pavan]

Dan,

 
----------------
Thanks & Regards,
Pavan Savoy | x0099669
 
> -----Original Message-----
> From: Dan Carpenter [mailto:error27 at gmail.com]
> Sent: Friday, August 27, 2010 8:52 AM
> To: Savoy, Pavan
> Cc: devel at driverdev.osuosl.org
> Subject: [bug report] ti-st: potential overflow calling st_send_frame()
> 
> Hi Pavan,
> 
> I was doing an audit for buffer overflows and I think there may be a
> problem in the ti-st driver.
> 
> drivers/staging/ti-st/st_core.c +269
> 	st_int_recv(45) error: buffer overflow calling st_send_frame. param 0.
> 3 >= 3
>    264                          case ST_BT_W4_DATA:
>    265                                  pr_debug("Complete pkt received");
>    266
>    267                                  /* Ask ST CORE to forward
>    268                                   * the packet to protocol driver */
>    269                                  st_send_frame(protoid, st_gdata);
>                                                       ^^^^^^^
> I think this could be equal to ST_MAX (3) here.
> 
>    270
>    271                                  st_gdata->rx_state =
> ST_W4_PACKET_TYPE;
>    272                                  st_gdata->rx_skb = NULL;
>    273                                  protoid = ST_MAX;       /* is this
> required ? */
> 
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> In st_send_frame() we check st_gdata->list[protoid] but ->list only has
> ST_MAX elements so if protoid is ST_MAX we are one past the end of the
> array.

I am not sure I understand, the logic is something like this, when the state of the recv is W4_DATA, the protoid would be already set to either ST_BT, ST_FM or ST_GPS based on the packet id received under state HCI_EVENT_PKT or ST_FM_CH8_PKT or 0x9.
The protoid is set to ST_MAX after doing a st_send_frame because the protoid variable needs to be sort of reset (not absolutely necessary), but the state has to be carefully updated to W4_PACKET_TYPE so that protoid is set to right value upon recv-ing next packet..

This is all because the UART may send in 100 bytes of BT data in chunks of 5 20bytes packets.

Does this make sense?




> regards,
> dan carpenter