From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 93640751BA for ; Wed, 23 Jun 2021 15:18:27 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 88D2EB38A for ; Wed, 23 Jun 2021 15:18:27 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id E5F9CB378 for ; Wed, 23 Jun 2021 15:18:23 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id AB689457EA for ; Wed, 23 Jun 2021 15:18:23 +0200 (CEST) Date: Wed, 23 Jun 2021 15:18:21 +0200 From: Stoiko Ivanov To: Thomas Lamprecht Cc: Proxmox VE development discussion Message-ID: <20210623151821.1cbabf05@rosa.proxmox.com> In-Reply-To: <49e4b2cc-5c15-b343-e374-e7d78f6c69ac@proxmox.com> References: <20210622163954.5580-1-s.ivanov@proxmox.com> <49e4b2cc-5c15-b343-e374-e7d78f6c69ac@proxmox.com> X-Mailer: Claws Mail 3.17.8 (GTK+ 2.24.33; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.641 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] applied: [PATCH storage] plugins: untaint volume_size_info retuns X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 23 Jun 2021 13:18:27 -0000 On Wed, 23 Jun 2021 09:13:40 +0200 Thomas Lamprecht 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 > > --- > > > > 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)