jcs.org / qemu
qemu with hax to log dma reads & writes jcs.org/2018/11/12/vfio
fork atom

hw/scsi/megasas: Fix possible out-of-bounds array access in tracepoints

Some tracepoints in megasas.c use a guest-controlled value as an index
into the mfi_frame_desc[] array. Thus a malicious guest could cause an
out-of-bounds error here. Fortunately, the impact is very low since this
can only happen when the corresponding tracepoints have been enabled
before, but the problem should be fixed anyway with a proper check.

Buglink: https://bugs.launchpad.net/qemu/+bug/1882065
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20200615072629.32321-1-thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

authored by

Thomas Huth and committed by
Paolo Bonzini ee760ac8 5f509751

+23 -13
+23 -13
hw/scsi/megasas.c
··· 54 54 #define MEGASAS_FLAG_USE_QUEUE64 1 55 55 #define MEGASAS_MASK_USE_QUEUE64 (1 << MEGASAS_FLAG_USE_QUEUE64) 56 56 57 - static const char *mfi_frame_desc[] = { 58 - "MFI init", "LD Read", "LD Write", "LD SCSI", "PD SCSI", 59 - "MFI Doorbell", "MFI Abort", "MFI SMP", "MFI Stop"}; 60 - 61 57 typedef struct MegasasCmd { 62 58 uint32_t index; 63 59 uint16_t flags; ··· 181 177 { 182 178 PCIDevice *pci = &s->parent_obj; 183 179 stb_pci_dma(pci, frame + offsetof(struct mfi_frame_header, scsi_status), v); 180 + } 181 + 182 + static inline const char *mfi_frame_desc(unsigned int cmd) 183 + { 184 + static const char *mfi_frame_descs[] = { 185 + "MFI init", "LD Read", "LD Write", "LD SCSI", "PD SCSI", 186 + "MFI Doorbell", "MFI Abort", "MFI SMP", "MFI Stop" 187 + }; 188 + 189 + if (cmd < ARRAY_SIZE(mfi_frame_descs)) { 190 + return mfi_frame_descs[cmd]; 191 + } 192 + 193 + return "Unknown"; 184 194 } 185 195 186 196 /* ··· 1670 1680 if (is_logical) { 1671 1681 if (target_id >= MFI_MAX_LD || lun_id != 0) { 1672 1682 trace_megasas_scsi_target_not_present( 1673 - mfi_frame_desc[frame_cmd], is_logical, target_id, lun_id); 1683 + mfi_frame_desc(frame_cmd), is_logical, target_id, lun_id); 1674 1684 return MFI_STAT_DEVICE_NOT_FOUND; 1675 1685 } 1676 1686 } 1677 1687 sdev = scsi_device_find(&s->bus, 0, target_id, lun_id); 1678 1688 1679 1689 cmd->iov_size = le32_to_cpu(cmd->frame->header.data_len); 1680 - trace_megasas_handle_scsi(mfi_frame_desc[frame_cmd], is_logical, 1690 + trace_megasas_handle_scsi(mfi_frame_desc(frame_cmd), is_logical, 1681 1691 target_id, lun_id, sdev, cmd->iov_size); 1682 1692 1683 1693 if (!sdev || (megasas_is_jbod(s) && is_logical)) { 1684 1694 trace_megasas_scsi_target_not_present( 1685 - mfi_frame_desc[frame_cmd], is_logical, target_id, lun_id); 1695 + mfi_frame_desc(frame_cmd), is_logical, target_id, lun_id); 1686 1696 return MFI_STAT_DEVICE_NOT_FOUND; 1687 1697 } 1688 1698 1689 1699 if (cdb_len > 16) { 1690 1700 trace_megasas_scsi_invalid_cdb_len( 1691 - mfi_frame_desc[frame_cmd], is_logical, 1701 + mfi_frame_desc(frame_cmd), is_logical, 1692 1702 target_id, lun_id, cdb_len); 1693 1703 megasas_write_sense(cmd, SENSE_CODE(INVALID_OPCODE)); 1694 1704 cmd->frame->header.scsi_status = CHECK_CONDITION; ··· 1706 1716 cmd->req = scsi_req_new(sdev, cmd->index, lun_id, cdb, cmd); 1707 1717 if (!cmd->req) { 1708 1718 trace_megasas_scsi_req_alloc_failed( 1709 - mfi_frame_desc[frame_cmd], target_id, lun_id); 1719 + mfi_frame_desc(frame_cmd), target_id, lun_id); 1710 1720 megasas_write_sense(cmd, SENSE_CODE(NO_SENSE)); 1711 1721 cmd->frame->header.scsi_status = BUSY; 1712 1722 s->event_count++; ··· 1751 1761 } 1752 1762 1753 1763 trace_megasas_handle_io(cmd->index, 1754 - mfi_frame_desc[frame_cmd], target_id, lun_id, 1764 + mfi_frame_desc(frame_cmd), target_id, lun_id, 1755 1765 (unsigned long)lba_start, (unsigned long)lba_count); 1756 1766 if (!sdev) { 1757 1767 trace_megasas_io_target_not_present(cmd->index, 1758 - mfi_frame_desc[frame_cmd], target_id, lun_id); 1768 + mfi_frame_desc(frame_cmd), target_id, lun_id); 1759 1769 return MFI_STAT_DEVICE_NOT_FOUND; 1760 1770 } 1761 1771 1762 1772 if (cdb_len > 16) { 1763 1773 trace_megasas_scsi_invalid_cdb_len( 1764 - mfi_frame_desc[frame_cmd], 1, target_id, lun_id, cdb_len); 1774 + mfi_frame_desc(frame_cmd), 1, target_id, lun_id, cdb_len); 1765 1775 megasas_write_sense(cmd, SENSE_CODE(INVALID_OPCODE)); 1766 1776 cmd->frame->header.scsi_status = CHECK_CONDITION; 1767 1777 s->event_count++; ··· 1781 1791 lun_id, cdb, cmd); 1782 1792 if (!cmd->req) { 1783 1793 trace_megasas_scsi_req_alloc_failed( 1784 - mfi_frame_desc[frame_cmd], target_id, lun_id); 1794 + mfi_frame_desc(frame_cmd), target_id, lun_id); 1785 1795 megasas_write_sense(cmd, SENSE_CODE(NO_SENSE)); 1786 1796 cmd->frame->header.scsi_status = BUSY; 1787 1797 s->event_count++;