Re: [Qemu-block] Overflow in Virtio-BLK and SCSI Requests?

[P J P]

  Hello Stefan, all

+-- On Mon, 30 May 2016, Peter Lieven wrote --+
| Am 27.05.2016 um 23:22 schrieb Stefan Hajnoczi:
| > On Fri, May 20, 2016 at 11:27:02AM +0200, Peter Lieven wrote:
| > > while working at the iSCSI code in Qemu I came across the following line
| > > in iscsi_aio_ioctl
| > >
| > > memcpy(&acb->task->cdb[0], acb->ioh->cmdp, acb->ioh->cmd_len);
| > >
| > > Is there anything to ensure that the cmd_len is valid when the requests is
| > > e.g. coming in via
| > > virtio_blk_handle_scsi ?
| > >
| > > It seems that virtio-scsi does not allow to pass ioctls directly from
| > > Guest, but at least virtio-blk
| > > does. And in virtio-blk it seems the data is blindly copied from
| > > elem->out_sg[1]. So it would
| > > be possible to overflow the acb->task->cdb. Or am I wrong here?
| > I agree that the guest can trigger a buffer overflow here.
| >
| > I think the bug is in iscsi_aio_ioctl() because ioctl handlers must
| > validates their inputs.  iscsi.c assumes acb->ioh->cmd_len is always
| > less than sizeof(acb->task->cdb[]) (SCSI_CDB_MAX_SIZE).
| >
| > iscsi_aio_ioctl() needs to be audited and checks should be added for
| > assumptions like this.
| >
| > Do you have time to do this?
| 
| I already submitted a patch and Paolo has put it in his queue.
| 
|  block/iscsi: avoid potential overflow of acb->task->cdb

   Great! Thank you so much for CC'ing me. I'll process it further.

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F