[pve-devel] [PATCH storage 0/2] LIO: fix tainted config and minor cleanup

[pve-devel] [PATCH storage 0/2] LIO: fix tainted config and minor cleanup

[Stoiko Ivanov]

This patchset follows the fix in 609f117ff24d2cff6b155e1d4b1175ceebe5bd7b.
Since several operations rely on values read from the remote target's
LIO config (which is tainted), this patchset untaints the settings, which are
actually needed (and stores only those), while reading the config.

The second patch removes two small left-over statements, which confused me
while debugging.

Tested very roughly (adding disks, create/rollback snapshot, remove disk) on
a VM backed by a LIO-iSCSI target.

Stoiko Ivanov (2):
  LIO: untaint values read from remote config
  LIO: drop unused statements

 PVE/Storage/LunCmd/LIO.pm | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

-- 
2.20.1

[pve-devel] [PATCH storage 1/2] LIO: untaint values read from remote config

[Stoiko Ivanov]

The LIO backend for ZFS over iSCSI fetches the json-config periodically from
the target.
This patch reduces the stored config values to those which are actually used
and additonally untaints the values read from the remote host's config-file.

Since the LUN index is used in calls to targetcli on the remote host (via
run_command), untainting prevents the call to crash when run with '-T'.

Tested by creating a zfs over iscsi backed VM, starting it, adding disks,
resizing disks, removing disks, creating snapshots, rolling back to a snapshot.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 PVE/Storage/LunCmd/LIO.pm | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/PVE/Storage/LunCmd/LIO.pm b/PVE/Storage/LunCmd/LIO.pm
index 15ddabf..f9e7143 100644
--- a/PVE/Storage/LunCmd/LIO.pm
+++ b/PVE/Storage/LunCmd/LIO.pm
@@ -154,8 +154,22 @@ my $parser = sub {
 	# find correct TPG
 	foreach my $tpg (@{$target->{tpgs}}) {
 	    if ($tpg->{tag} == $tpg_tag) {
+		my $res = [];
+		foreach my $lun (@{$tpg->{luns}}) {
+		    my ($idx, $storage_object);
+		    if ($lun->{index} =~ /^(\d+)$/) {
+			$idx = $1;
+		    }
+		    if ($lun->{storage_object} =~ m|^($BACKSTORE/.*)$|) {
+			$storage_object = $1;
+		    }
+		    die "Invalid lun definition in config!\n"
+			if !(defined($idx) && defined($storage_object));
+		    push @$res, { index => $idx, storage_object => $storage_object };
+		}
+
 		my $id = "$scfg->{portal}.$scfg->{target}";
-		$SETTINGS->{$id} = $tpg;
+		$SETTINGS->{$id}->{luns} = $res;
 		$haveTarget = 1;
 		last;
 	    }
-- 
2.20.1

[pve-devel] [PATCH storage 2/2] LIO: drop unused statements

[Stoiko Ivanov]

minor cleanup of left-over/unused statements.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 PVE/Storage/LunCmd/LIO.pm | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/PVE/Storage/LunCmd/LIO.pm b/PVE/Storage/LunCmd/LIO.pm
index f9e7143..b98edc2 100644
--- a/PVE/Storage/LunCmd/LIO.pm
+++ b/PVE/Storage/LunCmd/LIO.pm
@@ -142,8 +142,6 @@ my $parser = sub {
 	die "Target Portal Group has invalid value, must contain string 'tpg' and a suffix number, eg 'tpg17'\n";
     }
 
-    my $base = get_base;
-
     my $config = $get_config->($scfg);
     my $jsonconfig = JSON->new->utf8->decode($config);
 
@@ -266,7 +264,6 @@ my $list_view = sub {
 # determines, if the given object exists on the portal
 my $list_lun = sub {
     my ($scfg, $timeout, $method, @params) = @_;
-    my $name = undef;
 
     my $object = $params[0];
     my $volname = $extract_volname->($scfg, $object);
@@ -278,7 +275,7 @@ my $list_lun = sub {
 	}
     }
 
-    return $name;
+    return undef;
 };
 
 # adds a new LUN to the target
-- 
2.20.1

Re: [pve-devel] [PATCH storage 0/2] LIO: fix tainted config and minor cleanup

[Daniel Berteaud]

----- Le 12 Oct 20, à 17:34, Stoiko Ivanov s.ivanov@proxmox.com a écrit :

> This patchset follows the fix in 609f117ff24d2cff6b155e1d4b1175ceebe5bd7b.
> Since several operations rely on values read from the remote target's
> LIO config (which is tainted), this patchset untaints the settings, which are
> actually needed (and stores only those), while reading the config.
> 
> The second patch removes two small left-over statements, which confused me
> while debugging.
> 
> Tested very roughly (adding disks, create/rollback snapshot, remove disk) on
> a VM backed by a LIO-iSCSI target.
> 


Just tested it on my install, and everything seems to be working again (tested disk creation, removal, resize, move, snapshot, rollback, delete snapshot)

Thanks !

-- 
[ https://www.firewall-services.com/ ] 	
Daniel Berteaud 
FIREWALL-SERVICES SAS, La sécurité des réseaux 
Société de Services en Logiciels Libres 
Tél : +33.5 56 64 15 32 
Matrix: @dani:fws.fr 
[ https://www.firewall-services.com/ | https://www.firewall-services.com ]

[pve-devel] applied-series: [PATCH storage 0/2] LIO: fix tainted config and minor cleanup

[Thomas Lamprecht]

On 12.10.20 17:34, Stoiko Ivanov wrote:
> 
> This patchset follows the fix in 609f117ff24d2cff6b155e1d4b1175ceebe5bd7b.
> Since several operations rely on values read from the remote target's
> LIO config (which is tainted), this patchset untaints the settings, which are
> actually needed (and stores only those), while reading the config.
> 
> The second patch removes two small left-over statements, which confused me
> while debugging.
> 
> Tested very roughly (adding disks, create/rollback snapshot, remove disk) on
> a VM backed by a LIO-iSCSI target.
> 
> Stoiko Ivanov (2):
>   LIO: untaint values read from remote config
>   LIO: drop unused statements
> 
>  PVE/Storage/LunCmd/LIO.pm | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 



applied series, thanks!