Am 20.11.2014 um 21:08 hat Dr. David Alan Gilbert geschrieben:
> * Kevin Wolf (kw...@redhat.com) wrote:
>
>
> > diff --git a/block/raw_bsd.c b/block/raw_bsd.c
> > index 401b967..2ce5409 100644
> > --- a/block/raw_bsd.c
> > +++ b/block/raw_bsd.c
> > @@ -58,8 +58,58 @@ static int coroutine_fn raw_co_readv(BlockDriverState
> > *bs, int64_t sector_num,
> > static int coroutine_fn raw_co_writev(BlockDriverState *bs, int64_t
> > sector_num,
> > int nb_sectors, QEMUIOVector *qiov)
> > {
> > + void *buf = NULL;
> > + BlockDriver *drv;
> > + QEMUIOVector local_qiov;
> > + int ret;
> > +
> > + if (bs->probed && sector_num == 0) {
> > + /* As long as these conditions are true, we can't get partial
> > writes to
> > + * the probe buffer and can just directly check the request. */
> > + QEMU_BUILD_BUG_ON(BLOCK_PROBE_BUF_SIZE != 512);
> > + QEMU_BUILD_BUG_ON(BDRV_SECTOR_SIZE != 512);
> > +
> > + if (nb_sectors == 0) {
> > + /* qemu_iovec_to_buf() would fail, but we want to return
> > success
> > + * instead of -EINVAL in this case. */
> > + return 0;
> > + }
> > +
> > + buf = qemu_try_blockalign(bs->file, 512);
> > + if (!buf) {
> > + ret = -ENOMEM;
> > + goto fail;
> > + }
> > +
> > + ret = qemu_iovec_to_buf(qiov, 0, buf, 512);
> > + if (ret != 512) {
> > + ret = -EINVAL;
> > + goto fail;
> > + }
> > +
> > + drv = bdrv_probe_all(buf, 512, NULL);
> > + if (drv != bs->drv) {
> > + ret = -EPERM;
> > + goto fail;
> > + }
>
> Two things about this worry me:
> 1) It allows a running guest to prod at the probing code potentially quite
> hard; if there is anything nasty that can be done during probing it would
> potentially make it easier for a guest to find it.
The probing functions are trivial. You can audit them in no time even
with no previous block layer experience. They just do a few tests on the
passed buffer.
> 2) We don't log anything when this failure happens so if someone hits
> this by accident for some reason it'll confuse them no end. Could we add
> a (1 time?) error_report/printf just so that there's something to work with ?
We already log a warning on bdrv_open(). Don't you think that should be
enough?
Kevin
