gwj / at91sam9263 · Commits

Commit 71d0770c4cec9f1dc04f4dadcbf7fd6c335030a9

Authored by aliguori
1 parent b42ec42d

Fix CVE-2008-0928 - insufficient block device address range checking (Anthony Liguori) 

Introduce a growable flag that's set by bdrv_file_open().  Block devices should
never be growable, only files that are being used by block devices.

I went through Fabrice's early comments about the patch that was first applied.
While I disagree with that patch, I also disagree with Fabrice's suggestion.

There's no good reason to do the checks in the block drivers themselves.  It
just increases the possibility that this bug could show up again.  Since we're
calling bdrv_getlength() to determine the length, we're giving the block drivers
a chance to chime in and let us know what range is valid.

Basically, this patch makes the BlockDriver API guarantee that all requests are
within 0..bdrv_getlength() which to me seems like a Good Thing.

What do others think?

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>


git-svn-id: svn://svn.savannah.nongnu.org/qemu/trunk@6677 c046a42c-6fe2-441c-8c8c-71466251a162
Inline Side-by-side
Showing 2 changed files with 58 additions and 0 deletions
... ... @@ -318,6 +318,7 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename, int flags)
318 318 bdrv_delete(bs);
319 319 return ret;
320 320 }
  321 + bs->growable = 1;
321 322 *pbs = bs;
322 323 return 0;
323 324 }
... ... @@ -519,6 +520,39 @@ int bdrv_commit(BlockDriverState *bs)
519 520 return 0;
520 521 }
521 522  
  523 +static int bdrv_check_byte_request(BlockDriverState *bs, int64_t offset,
  524 + size_t size)
  525 +{
  526 + int64_t len;
  527 +
  528 + if (!bdrv_is_inserted(bs))
  529 + return -ENOMEDIUM;
  530 +
  531 + if (bs->growable)
  532 + return 0;
  533 +
  534 + len = bdrv_getlength(bs);
  535 +
  536 + if ((offset + size) > len)
  537 + return -EIO;
  538 +
  539 + return 0;
  540 +}
  541 +
  542 +static int bdrv_check_request(BlockDriverState *bs, int64_t sector_num,
  543 + int nb_sectors)
  544 +{
  545 + int64_t offset;
  546 +
  547 + /* Deal with byte accesses */
  548 + if (sector_num < 0)
  549 + offset = -sector_num;
  550 + else
  551 + offset = sector_num * 512;
  552 +
  553 + return bdrv_check_byte_request(bs, offset, nb_sectors * 512);
  554 +}
  555 +
522 556 /* return < 0 if error. See bdrv_write() for the return codes */
523 557 int bdrv_read(BlockDriverState *bs, int64_t sector_num,
524 558 uint8_t *buf, int nb_sectors)
... ... @@ -527,6 +561,8 @@ int bdrv_read(BlockDriverState *bs, int64_t sector_num,
527 561  
528 562 if (!drv)
529 563 return -ENOMEDIUM;
  564 + if (bdrv_check_request(bs, sector_num, nb_sectors))
  565 + return -EIO;
530 566  
531 567 if (drv->bdrv_pread) {
532 568 int ret, len;
... ... @@ -560,6 +596,9 @@ int bdrv_write(BlockDriverState *bs, int64_t sector_num,
560 596 return -ENOMEDIUM;
561 597 if (bs->read_only)
562 598 return -EACCES;
  599 + if (bdrv_check_request(bs, sector_num, nb_sectors))
  600 + return -EIO;
  601 +
563 602 if (drv->bdrv_pwrite) {
564 603 int ret, len, count = 0;
565 604 len = nb_sectors * 512;
... ... @@ -681,6 +720,9 @@ int bdrv_pread(BlockDriverState *bs, int64_t offset,
681 720  
682 721 if (!drv)
683 722 return -ENOMEDIUM;
  723 + if (bdrv_check_byte_request(bs, offset, count1))
  724 + return -EIO;
  725 +
684 726 if (!drv->bdrv_pread)
685 727 return bdrv_pread_em(bs, offset, buf1, count1);
686 728 return drv->bdrv_pread(bs, offset, buf1, count1);
... ... @@ -696,6 +738,9 @@ int bdrv_pwrite(BlockDriverState *bs, int64_t offset,
696 738  
697 739 if (!drv)
698 740 return -ENOMEDIUM;
  741 + if (bdrv_check_byte_request(bs, offset, count1))
  742 + return -EIO;
  743 +
699 744 if (!drv->bdrv_pwrite)
700 745 return bdrv_pwrite_em(bs, offset, buf1, count1);
701 746 return drv->bdrv_pwrite(bs, offset, buf1, count1);
... ... @@ -1299,6 +1344,9 @@ BlockDriverAIOCB *bdrv_aio_readv(BlockDriverState *bs, int64_t sector_num,
1299 1344 QEMUIOVector *iov, int nb_sectors,
1300 1345 BlockDriverCompletionFunc *cb, void *opaque)
1301 1346 {
  1347 + if (bdrv_check_request(bs, sector_num, nb_sectors))
  1348 + return NULL;
  1349 +
1302 1350 return bdrv_aio_rw_vector(bs, sector_num, iov, nb_sectors,
1303 1351 cb, opaque, 0);
1304 1352 }
... ... @@ -1307,6 +1355,9 @@ BlockDriverAIOCB *bdrv_aio_writev(BlockDriverState *bs, int64_t sector_num,
1307 1355 QEMUIOVector *iov, int nb_sectors,
1308 1356 BlockDriverCompletionFunc *cb, void *opaque)
1309 1357 {
  1358 + if (bdrv_check_request(bs, sector_num, nb_sectors))
  1359 + return NULL;
  1360 +
1310 1361 return bdrv_aio_rw_vector(bs, sector_num, iov, nb_sectors,
1311 1362 cb, opaque, 1);
1312 1363 }
... ... @@ -1320,6 +1371,8 @@ BlockDriverAIOCB *bdrv_aio_read(BlockDriverState *bs, int64_t sector_num,
1320 1371  
1321 1372 if (!drv)
1322 1373 return NULL;
  1374 + if (bdrv_check_request(bs, sector_num, nb_sectors))
  1375 + return NULL;
1323 1376  
1324 1377 ret = drv->bdrv_aio_read(bs, sector_num, buf, nb_sectors, cb, opaque);
1325 1378  
... ... @@ -1343,6 +1396,8 @@ BlockDriverAIOCB *bdrv_aio_write(BlockDriverState *bs, int64_t sector_num,
1343 1396 return NULL;
1344 1397 if (bs->read_only)
1345 1398 return NULL;
  1399 + if (bdrv_check_request(bs, sector_num, nb_sectors))
  1400 + return NULL;
1346 1401  
1347 1402 ret = drv->bdrv_aio_write(bs, sector_num, buf, nb_sectors, cb, opaque);
1348 1403  
... ...
block_int.h
  View file @71d0770
... ... @@ -121,6 +121,9 @@ struct BlockDriverState {
121 121 uint64_t rd_ops;
122 122 uint64_t wr_ops;
123 123  
  124 + /* Whether the disk can expand beyond total_sectors */
  125 + int growable;
  126 +
124 127 /* NOTE: the following infos are only hints for real hardware
125 128 drivers. They are not used by the block driver */
126 129 int cyls, heads, secs, translation;
... ...