From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <t.lamprecht@proxmox.com> 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) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 94B7B74E94 for <pve-devel@lists.proxmox.com>; Wed, 23 Jun 2021 09:13:59 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 870562E1AF for <pve-devel@lists.proxmox.com>; Wed, 23 Jun 2021 09:13:59 +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) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id AE08C2E19E for <pve-devel@lists.proxmox.com>; Wed, 23 Jun 2021 09:13:57 +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 76D5542FAD for <pve-devel@lists.proxmox.com>; Wed, 23 Jun 2021 09:13:51 +0200 (CEST) Message-ID: <49e4b2cc-5c15-b343-e374-e7d78f6c69ac@proxmox.com> Date: Wed, 23 Jun 2021 09:13:40 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:90.0) Gecko/20100101 Thunderbird/90.0 Content-Language: en-US To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>, Stoiko Ivanov <s.ivanov@proxmox.com> References: <20210622163954.5580-1-s.ivanov@proxmox.com> From: Thomas Lamprecht <t.lamprecht@proxmox.com> In-Reply-To: <20210622163954.5580-1-s.ivanov@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.684 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: [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 <pve-devel.lists.proxmox.com> List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe> List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/> List-Post: <mailto:pve-devel@lists.proxmox.com> List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help> List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe> X-List-Received-Date: Wed, 23 Jun 2021 07:13:59 -0000 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.ivanov@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.