gwj / at91sam9263 · Commits

Commit 274fb0e1ed962e9ae43ab05e7939499cebb39d26

Authored by aliguori
1 parent 33049de7

check SCSI read/write requests against max LBA (Rik van Riel) 

The bdrv layer uses a signed offset. Furthermore, block-raw-posix
only seeks when that offset is positive. Passing a negative offset
to block-raw-posix can result in data being written at the current
seek cursor's position.

It may be possible to exploit this to seek to the end of the disk
and extend the virtual disk by writing data to a negative sector
offset.  After a reboot, this could lead to the guest having a
larger disk than it had before.

Close the hole by sanity checking the lba against the size of the
disk.

Signed-off-by: Rik van Riel <riel@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>


git-svn-id: svn://svn.savannah.nongnu.org/qemu/trunk@6475 c046a42c-6fe2-441c-8c8c-71466251a162
Inline Side-by-side
Showing 1 changed file with 18 additions and 0 deletions
hw/scsi-disk.c
  View file @274fb0e
... ... @@ -67,6 +67,7 @@ struct SCSIDeviceState
67 67 /* The qemu block layer uses a fixed 512 byte sector size.
68 68 This is the number of 512 byte blocks in a single scsi sector. */
69 69 int cluster_size;
  70 + uint64_t max_lba;
70 71 int sense;
71 72 int tcq;
72 73 /* Completion functions may be called from either scsi_{read,write}_data
... ... @@ -738,6 +739,8 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag,
738 739 /* Returned value is the address of the last sector. */
739 740 if (nb_sectors) {
740 741 nb_sectors--;
  742 + /* Remember the new size for read/write sanity checking. */
  743 + s->max_lba = nb_sectors;
741 744 /* Clip to 2TB, instead of returning capacity modulo 2TB. */
742 745 if (nb_sectors > UINT32_MAX)
743 746 nb_sectors = UINT32_MAX;
... ... @@ -759,6 +762,8 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag,
759 762 case 0x28:
760 763 case 0x88:
761 764 DPRINTF("Read (sector %lld, count %d)\n", lba, len);
  765 + if (lba > s->max_lba)
  766 + goto illegal_lba;
762 767 r->sector = lba * s->cluster_size;
763 768 r->sector_count = len * s->cluster_size;
764 769 break;
... ... @@ -766,6 +771,8 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag,
766 771 case 0x2a:
767 772 case 0x8a:
768 773 DPRINTF("Write (sector %lld, count %d)\n", lba, len);
  774 + if (lba > s->max_lba)
  775 + goto illegal_lba;
769 776 r->sector = lba * s->cluster_size;
770 777 r->sector_count = len * s->cluster_size;
771 778 is_write = 1;
... ... @@ -839,6 +846,8 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag,
839 846 /* Returned value is the address of the last sector. */
840 847 if (nb_sectors) {
841 848 nb_sectors--;
  849 + /* Remember the new size for read/write sanity checking. */
  850 + s->max_lba = nb_sectors;
842 851 outbuf[0] = (nb_sectors >> 56) & 0xff;
843 852 outbuf[1] = (nb_sectors >> 48) & 0xff;
844 853 outbuf[2] = (nb_sectors >> 40) & 0xff;
... ... @@ -877,6 +886,9 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag,
877 886 fail:
878 887 scsi_command_complete(r, STATUS_CHECK_CONDITION, SENSE_ILLEGAL_REQUEST);
879 888 return 0;
  889 + illegal_lba:
  890 + scsi_command_complete(r, STATUS_CHECK_CONDITION, SENSE_HARDWARE_ERROR);
  891 + return 0;
880 892 }
881 893 if (r->sector_count == 0 && r->buf_len == 0) {
882 894 scsi_command_complete(r, STATUS_GOOD, SENSE_NO_SENSE);
... ... @@ -902,6 +914,7 @@ SCSIDevice *scsi_disk_init(BlockDriverState *bdrv, int tcq,
902 914 {
903 915 SCSIDevice *d;
904 916 SCSIDeviceState *s;
  917 + uint64_t nb_sectors;
905 918  
906 919 s = (SCSIDeviceState *)qemu_mallocz(sizeof(SCSIDeviceState));
907 920 s->bdrv = bdrv;
... ... @@ -913,6 +926,11 @@ SCSIDevice *scsi_disk_init(BlockDriverState *bdrv, int tcq,
913 926 } else {
914 927 s->cluster_size = 1;
915 928 }
  929 + bdrv_get_geometry(s->bdrv, &nb_sectors);
  930 + nb_sectors /= s->cluster_size;
  931 + if (nb_sectors)
  932 + nb_sectors--;
  933 + s->max_lba = nb_sectors;
916 934 strncpy(s->drive_serial_str, drive_get_serial(s->bdrv),
917 935 sizeof(s->drive_serial_str));
918 936 if (strlen(s->drive_serial_str) == 0)
... ...