On Wed, 23 Jun 2021 09:13:40 +0200 Thomas Lamprecht <t.lampre...@proxmox.com> wrote:
> On 22.06.21 18:39, Stoiko Ivanov wrote: > > the size returned by volume_size_info is used for creating the new > > destination image in PVE::QemuServer::clone_disk (and probably > > elsewhere). In certain cases the return values are tainted - they are > > obtained by a run_command call and depending on the format and length > > of the parsed output can still have their tainted attribute. > > > > One example of a tainted return has been reported in our > > community-forum: > > https://forum.proxmox.com/threads/cannot-clone-vm-or-move-disk-with-more-than-13-snapshots.89628/ > > > > A qcow2 image with 13 snapshots generates a output > 4k in length from > > `qemu-img info --output=json`, which in turn causes the output to be > > considered tainted. > > > > This patch untaints the returns where applicable. The other > > storage-plugins are not affected: > > * LVMPlugin returns a single number and a newline (thus gets untainted > > by run_command) > > * RBDPlugin untaints the complete json before decoding > > * ZFSPoolplugin and ISCSIDirectPlugin explicitly untaint their > > returns. > > > > Signed-off-by: Stoiko Ivanov <s.iva...@proxmox.com> > > --- > > > > Note: > > Not really a v2, since it's a different patch, but addresses the same issue > > as in https://lists.proxmox.com/pipermail/pve-devel/2021-June/048910.html > > Aren't the version rather tied to the issue they try to solve, as > implementations > and approaches can always significantly change? So I'd be OK with this being > marked as v2, but no hard feelings. > > > > > PVE/Storage/PBSPlugin.pm | 4 +++- > > PVE/Storage/Plugin.pm | 6 ++++++ > > 2 files changed, 9 insertions(+), 1 deletion(-) > > > > > > applied to master and a newly branched stable-6, thanks! > > I made two followups: > > 1) - fix nit that we have a space in comments after the # > - actually die with error message if untaint fails > > 2) return early if decode JSON fails, compensates issues where the qemu-img > command somehow fails (e.g., file was removed since the stat check) and > the semantic change from 1) > > Please check if this still looks alright to you. Thanks for the improvements - Look fine to me! (and survived a few quick tests of disk-moving) _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
