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) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id ACC9C96426 for ; Mon, 15 Apr 2024 17:03:34 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 935E9D1A4 for ; Mon, 15 Apr 2024 17:03:34 +0200 (CEST) Received: from nena.proxmox.com (unknown [94.136.29.99]) by firstgate.proxmox.com (Proxmox) with ESMTP for ; Mon, 15 Apr 2024 17:03:28 +0200 (CEST) Received: by nena.proxmox.com (Postfix, from userid 1000) id 2B9EC229053; Mon, 15 Apr 2024 17:03:28 +0200 (CEST) From: Mira Limbeck To: pve-devel@lists.proxmox.com Date: Mon, 15 Apr 2024 17:03:12 +0200 Message-Id: <20240415150311.204138-1-m.limbeck@proxmox.com> X-Mailer: git-send-email 2.39.2 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.615 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment KAM_LAZY_DOMAIN_SECURITY 1 Sending domain does not have any anti-forgery methods RDNS_NONE 0.793 Delivered to internal network by a host with no rDNS SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_NONE 0.001 SPF: sender does not publish an SPF Record Subject: [pve-devel] [PATCH storage] fix insecure migration failing if waiting on lock 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: Mon, 15 Apr 2024 15:03:34 -0000 both STDOUT and STDERR are written into `$info` which is then parsed for IP and port of the target socket listening. when the ports file can't be locked immediately `trying to acquire lock...` is printed on STDERR and in turn written into `$info`. trying to parse the IP then fails, resulting in a migration or replication failing. the bare open3 call is replaced by the run_command wrapper from pve-common to use a safe wrapper around open3 with the same functionality. STDERR is now read separately from STDOUT and the last line of STDERR is kept in case of errors. Signed-off-by: Mira Limbeck --- I've replaced open3 with run_command on recommendation from others. tt complicates the logic a little bit compared to the `simple` open3 solution, but may be less error prone since run_command abstracts open3 nicely behind a safe wrapper. one thing I could not verify was the now removed line: `if (!close($info)) { # does waitpid()` I could not find any information mentioning this behavior of close() on a simple file. src/PVE/Storage.pm | 79 +++++++++++++++++++++++++--------------------- 1 file changed, 43 insertions(+), 36 deletions(-) diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm index 40314a8..b6045d5 100755 --- a/src/PVE/Storage.pm +++ b/src/PVE/Storage.pm @@ -851,44 +851,51 @@ sub storage_migrate { eval { if ($insecure) { - my $input = IO::File->new(); - my $info = IO::File->new(); - open3($input, $info, $info, @$recv) - or die "receive command failed: $!\n"; - close($input); - - my $try_ip = <$info> // ''; - my ($ip) = $try_ip =~ /^($PVE::Tools::IPRE)$/ # untaint - or die "no tunnel IP received, got '$try_ip'\n"; - - my $try_port = <$info> // ''; - my ($port) = $try_port =~ /^(\d+)$/ # untaint - or die "no tunnel port received, got '$try_port'\n"; - - my $socket = IO::Socket::IP->new(PeerHost => $ip, PeerPort => $port, Type => SOCK_STREAM) - or die "failed to connect to tunnel at $ip:$port\n"; - # we won't be reading from the socket - shutdown($socket, 0); - - eval { run_command($cmds, output => '>&'.fileno($socket), errfunc => $match_volid_and_log); }; - my $send_error = $@; - - # don't close the connection entirely otherwise the receiving end - # might not get all buffered data (and fails with 'connection reset by peer') - shutdown($socket, 1); - - # wait for the remote process to finish - while (my $line = <$info>) { - $match_volid_and_log->("[$target_sshinfo->{name}] $line"); + my $last_err = ''; + my $ip; + my $port; + my $socket; + my $send_error; + my $handle_insecure_migration = sub { + my $line = shift; + + if (!$ip) { + if ($line =~ /^($PVE::Tools::IPRE)$/) { + $ip = $1; + } else { + die "no tunnel IP received, got $line\n"; + } + } elsif(!$port) { + if ($line =~ /^(\d+)$/) { + $port = $1; + } else { + die "no tunnel port received, got $line\n"; + } + } + if ($ip && $port && !$socket) { + # create socket, run command + $socket = IO::Socket::IP->new(PeerHost => $ip, PeerPort => $port, Type => SOCK_STREAM) + or die "failed to connect to tunnel at $ip:$port\n"; + # we won't be reading from the socket + shutdown($socket, 0); + + eval { run_command($cmds, output => '>&'.fileno($socket), errfunc => $match_volid_and_log); }; + $send_error = $@; + + # don't close the connection entirely otherwise the receiving end + # might not get all buffered data (and fails with 'connection reset by peer') + shutdown($socket, 1); + } elsif ($ip && $port) { + $match_volid_and_log->("[$target_sshinfo->{name}] $line"); + } + }; + eval { run_command($recv, outfunc => $handle_insecure_migration, errfunc => sub { $last_err = shift; }); }; + if (my $err = $@) { + chomp($err); + die "failed to run insecure migration: $err - $last_err\n"; } - # now close the socket - close($socket); - if (!close($info)) { # does waitpid() - die "import failed: $!\n" if $!; - die "import failed: exit code ".($?>>8)."\n"; - } - + close($socket) if $socket; die $send_error if $send_error; } else { push @$cmds, $recv; -- 2.39.2