Re: [pve-devel] [PATCH v2 container] fix #4192: revamp check for systemd version

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

Am 12/09/2022 um 14:41 schrieb Fabian Grünbichler:
>> Instead of iterating through several folders, it might just be easier to
>> check the ldd output of /sbin/init and getting the version from there.
>> Furthermore, the regex for checking the version has been adapted so that
>> it's more precise.
> ldd is not suited for this purpose for security reasons, since /sbin/init 
> is a user/attacker-controlled binary in this case and we are only in a 
> chroot while doing the setup, not really containerized. given a crafted 
> container template/backup archive/.. this could execute arbitrary code.
> 
> it's manpage suggests using
> 
>  objdump -p /path/to/binary
> 
> and looking at the lines with "NEEDED", which seems to me should be fine 
> for what we want to achieve here 😄
> 

tbf, I suggested using something like ldd here, but I mostly meant if we actually
need to further extend this than simply checking three instead of two paths.

But I actually like the much shorter code, so from that POV it could be a nicer
option, but it makes us dependent on actually executing code from the CT archive,
which also assumes the availability of something like ldd or objdump, which may
not be the case in all templates?

Two small nits w.r.t. to the v2 still inline.

Am 12/09/2022 um 14:25 schrieb Leo Nunner:
> +    my $version = undef;
> +    PVE::Tools::run_command(
> +	[
> +	    'ldd',
> +	    '/sbin/init'
> +	],

you can put the array ref in one line, e.g., with Fabian relayed manpage
suggestion it'd be fine to do:

    [ 'objdump', '-p', '/sbin/init' ],

> +	outfunc => sub {
> +	    my $line = shift;
> +	    if ($line =~ /^\s*libsystemd-shared-(\d+)(?:\.[a-zA-Z0-9]*)?\.so/) {
> +		$version = $1;
> +	    }},
> +	errmsg => "ldd on /sbin/init failed"

above is missing a trailing comma, which would ensure that any possible addition of an
option in the future won't need to touch an unrelated line.

> +    );
> +
> +    return $version;
>  }