diff options
author | dim <dim@FreeBSD.org> | 2016-12-18 14:31:11 +0000 |
---|---|---|
committer | dim <dim@FreeBSD.org> | 2016-12-18 14:31:11 +0000 |
commit | fdc96b1347709438b325ea86883c585db249ed3a (patch) | |
tree | bfc670e3769c02c67cbb523a6d15dee9551cab6d | |
parent | f482b957ebe70bfe30b5c19c12f3a6d83639ef68 (diff) | |
download | FreeBSD-src-fdc96b1347709438b325ea86883c585db249ed3a.zip FreeBSD-src-fdc96b1347709438b325ea86883c585db249ed3a.tar.gz |
MFC r310013 (by cperciva):
Check that blkfront devices have a non-zero number of sectors and a
non-zero sector size. Such a device would be a virtual disk of zero
bytes; clearly not useful, and not something we should try to attach.
As a fortuitous side effect, checking that these values are non-zero
here results in them not *becoming* zero later on the function. This
odd behaviour began with r309124 (clang 3.9.0) but is challenging to
debug; making any changes to this function whatsoever seems to affect
the llvm optimizer behaviour enough to make the unexpected zeroing of
the sector_size variable cease.
PR: 215209
Security: The potential for variables to unexpectedly become zero
has worrying consequences for security in general, but
not so much in this particular context.
MFC r310086:
In xbd_connect(), use correct scanf conversion specifiers for the
feature_barrier and feature_flush variables. Otherwise, adjacent
variables on the stack, such as sector_size, may be overwritten, with
disastrous results.
Note that I did not see a good reason to revert the addition of zero
checks introduced in r310013. Better safe than sorry.
PR: 215209
Tested by: royger
-rw-r--r-- | sys/dev/xen/blkfront/blkfront.c | 12 |
1 files changed, 10 insertions, 2 deletions
diff --git a/sys/dev/xen/blkfront/blkfront.c b/sys/dev/xen/blkfront/blkfront.c index 1ad2494..58ddd80 100644 --- a/sys/dev/xen/blkfront/blkfront.c +++ b/sys/dev/xen/blkfront/blkfront.c @@ -1239,19 +1239,27 @@ xbd_connect(struct xbd_softc *sc) xenbus_get_otherend_path(dev)); return; } + if ((sectors == 0) || (sector_size == 0)) { + xenbus_dev_fatal(dev, 0, + "invalid parameters from %s:" + " sectors = %lu, sector_size = %lu", + xenbus_get_otherend_path(dev), + sectors, sector_size); + return; + } err = xs_gather(XST_NIL, xenbus_get_otherend_path(dev), "physical-sector-size", "%lu", &phys_sector_size, NULL); if (err || phys_sector_size <= sector_size) phys_sector_size = 0; err = xs_gather(XST_NIL, xenbus_get_otherend_path(dev), - "feature-barrier", "%lu", &feature_barrier, + "feature-barrier", "%d", &feature_barrier, NULL); if (err == 0 && feature_barrier != 0) sc->xbd_flags |= XBDF_BARRIER; err = xs_gather(XST_NIL, xenbus_get_otherend_path(dev), - "feature-flush-cache", "%lu", &feature_flush, + "feature-flush-cache", "%d", &feature_flush, NULL); if (err == 0 && feature_flush != 0) sc->xbd_flags |= XBDF_FLUSH; |