Re: [Qemu-devel] [PATCH] lsi53c895a: check message length value

[Ameya More]

Hi Prasad,

Thanks for following up on this. While Mark and I reported this issue to 
you, it was actually discovered by Dejvau Security and they should 
receive credit for reporting this issue.
http://www.dejavusecurity.com

Thanks,
Ameya


On 10/25/2018 03:09 PM, P J P wrote:
> From: Prasad J Pandit <address@hidden>
>
> While writing a message in 'lsi_do_msgin', message length value
> in msg_len could be invalid, add check to avoid OOB access issue.
>
> Reported-by: Ameya More <address@hidden>
> Signed-off-by: Prasad J Pandit <address@hidden>
> ---
>   hw/scsi/lsi53c895a.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
> index d1e6534311..a266c5a113 100644
> --- a/hw/scsi/lsi53c895a.c
> +++ b/hw/scsi/lsi53c895a.c
> @@ -205,7 +205,7 @@ typedef struct {
>       /* Action to take at the end of a MSG IN phase.
>          0 = COMMAND, 1 = disconnect, 2 = DATA OUT, 3 = DATA IN.  */
>       int msg_action;
> -    int msg_len;
> +    uint8_t msg_len;
>       uint8_t msg[LSI_MAX_MSGIN_LEN];
>       /* 0 if SCRIPTS are running or stopped.
>        * 1 if a Wait Reselect instruction has been issued.
> @@ -861,12 +861,15 @@ static void lsi_do_status(LSIState *s)
>   
>   static void lsi_do_msgin(LSIState *s)
>   {
> -    int len;
> +    uint8_t len;
>       trace_lsi_do_msgin(s->dbc, s->msg_len);
>       s->sfbr = s->msg[0];
>       len = s->msg_len;
>       if (len > s->dbc)
>           len = s->dbc;
> +    if (len > LSI_MAX_MSGIN_LEN) {
> +        len = LSI_MAX_MSGIN_LEN;
> +    }
>       pci_dma_write(PCI_DEVICE(s), s->dnad, s->msg, len);
>       /* Linux drivers rely on the last byte being in the SIDL.  */
>       s->sidl = s->msg[len - 1];
> @@ -2114,7 +2117,7 @@ static const VMStateDescription vmstate_lsi_scsi = {
>           VMSTATE_INT32(carry, LSIState),
>           VMSTATE_INT32(status, LSIState),
>           VMSTATE_INT32(msg_action, LSIState),
> -        VMSTATE_INT32(msg_len, LSIState),
> +        VMSTATE_UINT8(msg_len, LSIState),
>           VMSTATE_BUFFER(msg, LSIState),
>           VMSTATE_INT32(waiting, LSIState),
>