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

[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?

Stefan

signature.asc
Description: PGP signature