[Libguestfs] nbdkit problem with cache/cow and unaligned sizes

[Richard W.M. Jones]

On Wed, May 15, 2019 at 03:11:42PM -0500, Eric Blake wrote:
> Right now, the cache and cow filters always round up requests to blksize
> boundaries (blksize for cache is dynamically determined at connection
> start, for cow is fixed as BLKSIZE). Which is fine for the bulk of the
> underlying file, but can cause problems when reading past EOF for a
> partial tail of an underlying plugin. We aren't validating that filter
> calls to next_ops are within bounds; and even if the plugin tolerates
> the past-EOF read, we aren't guaranteeing that the client will always
> read 0 bytes in the past-EOF tail.
> 
> Several ideas of fixing it, each with some drawbacks:
> + in cache/cow_get_size(), truncate the plugin's size down to blksize
> prior to calling blk_set_size() (renders the plugin's tail unusable)
> + reject serving images that aren't already aligned to blksize (avoids
> missing bytes or worrying about past-EOF slop, but can be mean, unless...)
> + document that for unaligned images, you can use --filter=cache
> --filter=truncate round-up=BLKSIZE, to let the truncate filter take care
> of our slop (doesn't play nicely with the fact that we can only use a
> filter once, if a user wants to also use --filter=truncate prior to
> --filter=cache)
> + rewrite both the cache/blk.c and cow/blk.c handlers to pay more
> attention to unaligned EOF (code duplication)
> + teach filters.c next_ops to auto-cap filter requests into valid ranges
> prior to calling into the next layer (trickier than it looks, especially
> if we later add NBD resize extension support)
> + others?

So I'll just make a quick comment on this:

For plugins, the server is careful to only send requests to the plugin
which are within the bounds of the image.  Therefore plugins can
simply assume that offset/count passed in to them are correct.

However if a bad filter is placed on top of a plugin it could
incorrectly send bad bounds requests to the plugin, which could cause
mayhem.  I tried to implement checking of parameters passed between
layers, and it's a lot harder than it seems (for architectural
reasons).

Therefore we currently rely on filters being written correctly.

If seems like if the cache/cow plugins don't do this then they are
buggy.  But I've not had time to look at this in detail.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top