Am 25.07.2013 um 00:01 hat Ian Main geschrieben: > On Wed, Jul 24, 2013 at 02:32:53PM -0600, Eric Blake wrote: > > On 07/24/2013 04:55 AM, Kevin Wolf wrote: > > > > > Unconditionally overriding format for NEW_IMAGE_MODE_EXISTING is > > > definitely wrong. It's the user's choice which COW format to use for the > > > backup image. There's no reason why it has to be the same format as the > > > image that is being backed up. > > > > > > Before, bs->drv->format_name was a default for the case where a new > > > image had to be created and no format was given; and the format of > > > existing images could be probed. This is still what makes most sense to > > > me. What's even the goal with this change? > > Actually I think that code is wrong. If we are using > NEW_IMAGE_MODE_EXISTING then format doesn't get used. We just end up > using bdrv_open() below to open the existing image. Format should not > be specified for an existing image.
That bdrv_open() gets drv passed, which is the block driver specified by format. If you pass NULL instead, bdrv_open() tries to probe the format, which is unreliable (raw is indistinguishable from other formats) and as Eric notes has led to security problems in the past. > > Furthermore, I'm proposing that for 1.6, we should make the format > > argument mandatory for drive-backup. We made it optional for > > drive-mirror, to allow for probing, but there have been CVEs in the past > > due to probing of a raw file gone wrong. We can always relax a > > mandatory argument into an optional one in 1.7, if we decide that > > probing can be done safely, but we can never turn an optional argument > > into a mandatory one once the initial release bakes in the option. It > > would make the code a lot simpler to just have a mandatory format > > argument, instead of having to bake in and document hueristics on which > > format is picked when the caller doesn't provide one. > > So I made format mandatory in the last patch but only for > NEW_IMAGE_MODE_ABSOLUTE_PATHS. It actually doesn't make sense to > specify the format of an existing image so I left it optional as an > argument, but it will throw an error if it's not specified for the case > where we create a new image. > > That make sense? No, see above, it's important as which format an existing image is opened. Kevin
