From my perspective r->buflen can be more than 16 bytes, The Block limits VPD
page length is 0x3c (paragraph 5.4.5 page 475 from SCSI Commands Reference
Manual, Rev. J).
Absolutely you're right. What a mess. :)
Paolo
On Mon, May 15, 2023 at 3:58 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> Using linux 6.x guest, at boot time, an inquiry on a scsi-generic
> device makes qemu crash. This is caused by a buffer overflow when
> scsi-generic patches the block limits VPD page.
>
> Do the operations on a temporary on-stack buffer that is guaranteed
> to be large enough.
>
> Reported-by: Théo Maillart <tmaillart@freebox.fr>
> Analyzed-by: Théo Maillart <tmaillart@freebox.fr>
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> hw/scsi/scsi-generic.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
> index ac9fa662b4e3..373fab0a2a61 100644
> --- a/hw/scsi/scsi-generic.c
> +++ b/hw/scsi/scsi-generic.c
> @@ -191,12 +191,15 @@ static int scsi_handle_inquiry_reply(SCSIGenericReq *r, SCSIDevice *s, int len)
> if ((s->type == TYPE_DISK || s->type == TYPE_ZBC) &&
> (r->req.cmd.buf[1] & 0x01)) {
> page = r->req.cmd.buf[2];
> - if (page == 0xb0) {
> + if (page == 0xb0 && r->buflen >= 8) {
r->buflen > 8 because if r->buflen == 8, the final memcpy will be vain ?
> + uint8_t buf[16] = {};
> uint64_t max_transfer = calculate_max_transfer(s);
> - stl_be_p(&r->buf[8], max_transfer);
> - /* Also take care of the opt xfer len. */
> - stl_be_p(&r->buf[12],
> - MIN_NON_ZERO(max_transfer, ldl_be_p(&r->buf[12])));
> +
> + memcpy(buf, r->buf, r->buflen);
Should be memcpy(buf, r->buf, MIN(r->buflen, 16)); ?
> + stl_be_p(&buf[8], max_transfer);
> + stl_be_p(&buf[12], MIN_NON_ZERO(max_transfer, ldl_be_p(&buf[12])));
> + memcpy(r->buf + 8, buf + 8, r->buflen - 8);
Idem memcpy(r->buf + 8, buf + 8, MIN(r->buflen - 8, 8)); ?
> +
> } else if (s->needs_vpd_bl_emulation && page == 0x00 && r->buflen >= 4) {
> /*
> * Now we're capable of supplying the VPD Block Limits
> --
> 2.40.1
>