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