Currently, rebase interprets a relative path for the new backing image
as follows:
(1) Open the new backing image with the given relative path (thus relative to
qemu-img's working directory).
(2) Write it directly into the overlay's backing path field (thus
relative to the overlay).
If the overlay is not in qemu-img's working directory, both will be
different interpretations, which may either lead to an error somewhere
(either rebase fails because it cannot open the new backing image, or
your overlay becomes unusable because its backing path does not point
to a file), or, even worse, it may result in your rebase being
performed for a different backing file than what your overlay will point
to after the rebase.
Fix this by interpreting the target backing path as relative to the
overlay, like qemu-img does everywhere else.
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1569835
Cc: qemu-stable@nongnu.org
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
qemu-img.c | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/qemu-img.c b/qemu-img.c
index ea62d2d61e..50540519e3 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3191,6 +3191,9 @@ static int img_rebase(int argc, char **argv)
}
if (out_baseimg[0]) {
+ const char *overlay_filename;
+ char *out_real_path;
+
options = qdict_new();
if (out_basefmt) {
qdict_put_str(options, "driver", out_basefmt);
@@ -3199,8 +3202,25 @@ static int img_rebase(int argc, char **argv)
qdict_put_bool(options, BDRV_OPT_FORCE_SHARE, true);
}
- blk_new_backing = blk_new_open(out_baseimg, NULL,
+ overlay_filename = bs->exact_filename[0] ? bs->exact_filename
+ : bs->filename;
+ out_real_path = g_malloc(PATH_MAX);
+
+ bdrv_get_full_backing_filename_from_filename(overlay_filename,
+ out_baseimg,
+ out_real_path,
+ PATH_MAX,
+ &local_err);
+ if (local_err) {
+ error_reportf_err(local_err,
+ "Could not resolve backing filename: ");
+ ret = -1;
+ goto out;
+ }
+
+ blk_new_backing = blk_new_open(out_real_path, NULL,
options, src_flags, &local_err);
+ g_free(out_real_path);
if (!blk_new_backing) {
error_reportf_err(local_err,
"Could not open new backing file '%s': ",
--
2.14.3
On 05/09/2018 10:49 AM, Max Reitz wrote:
> Currently, rebase interprets a relative path for the new backing image
> as follows:
> (1) Open the new backing image with the given relative path (thus relative to
> qemu-img's working directory).
> (2) Write it directly into the overlay's backing path field (thus
> relative to the overlay).
>
> If the overlay is not in qemu-img's working directory, both will be
> different interpretations, which may either lead to an error somewhere
> (either rebase fails because it cannot open the new backing image, or
> your overlay becomes unusable because its backing path does not point
> to a file), or, even worse, it may result in your rebase being
> performed for a different backing file than what your overlay will point
> to after the rebase.
>
> Fix this by interpreting the target backing path as relative to the
> overlay, like qemu-img does everywhere else.
>
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1569835
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> - blk_new_backing = blk_new_open(out_baseimg, NULL,
> + overlay_filename = bs->exact_filename[0] ? bs->exact_filename
> + : bs->filename;
> + out_real_path = g_malloc(PATH_MAX);
> +
> + bdrv_get_full_backing_filename_from_filename(overlay_filename,
> + out_baseimg,
> + out_real_path,
> + PATH_MAX,
> + &local_err);
> + if (local_err) {
> + error_reportf_err(local_err,
> + "Could not resolve backing filename: ");
> + ret = -1;
> + goto out;
Leaks out_real_path.
> + }
> +
> + blk_new_backing = blk_new_open(out_real_path, NULL,
> options, src_flags, &local_err);
> + g_free(out_real_path);
Otherwise looks good.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
On 2018-05-09 19:20, Eric Blake wrote:
> On 05/09/2018 10:49 AM, Max Reitz wrote:
>> Currently, rebase interprets a relative path for the new backing image
>> as follows:
>> (1) Open the new backing image with the given relative path (thus
>> relative to
>> qemu-img's working directory).
>> (2) Write it directly into the overlay's backing path field (thus
>> relative to the overlay).
>>
>> If the overlay is not in qemu-img's working directory, both will be
>> different interpretations, which may either lead to an error somewhere
>> (either rebase fails because it cannot open the new backing image, or
>> your overlay becomes unusable because its backing path does not point
>> to a file), or, even worse, it may result in your rebase being
>> performed for a different backing file than what your overlay will point
>> to after the rebase.
>>
>> Fix this by interpreting the target backing path as relative to the
>> overlay, like qemu-img does everywhere else.
>>
>> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1569835
>> Cc: qemu-stable@nongnu.org
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>
>> - blk_new_backing = blk_new_open(out_baseimg, NULL,
>> + overlay_filename = bs->exact_filename[0] ?
>> bs->exact_filename
>> + : bs->filename;
>> + out_real_path = g_malloc(PATH_MAX);
>> +
>> +
>> bdrv_get_full_backing_filename_from_filename(overlay_filename,
>> + out_baseimg,
>> + out_real_path,
>> + PATH_MAX,
>> + &local_err);
>> + if (local_err) {
>> + error_reportf_err(local_err,
>> + "Could not resolve backing
>> filename: ");
>> + ret = -1;
>> + goto out;
>
> Leaks out_real_path.
Well, “leaks” is relative, considering that out: will effectively exit
the process immediately, but I can’t claim it was on purpose. O:-)
(I should have trusted my gut feeling that the single g_free() can’t
possibly be enough...)
Max
>> + }
>> +
>> + blk_new_backing = blk_new_open(out_real_path, NULL,
>> options, src_flags,
>> &local_err);
>> + g_free(out_real_path);
>
> Otherwise looks good.
>
© 2016 - 2025 Red Hat, Inc.