Bug#1011191: dpkg: let buildinfo record whether host architecture binaries can be executed when cross-compiling

2022-06-04 Thread Johannes Schauer Marin Rodrigues
Hi,

let me put the two relevant lists back into the CC because the members of those
lists can probably give better input than I.

Quoting Guillem Jover (2022-05-28 19:09:10)
> On Wed, 2022-05-18 at 07:26:02 +0200, Johannes Schauer Marin Rodrigues wrote:
> > Package: dpkg
> > Version: 1.21.7
> > Severity: wishlist
> > Tags: patch
> > X-Debbugs-Cc: jo...@debian.org
> 
> > when cross compiling, one property of the build system that can
> > influence the contents of the generate binary packages is whether or not
> > the host architecture can be executed. While some platforms can natively
> > execute others (like amd64 can execute i386), other combinations are
> > more surprising. When installing the qemu-user-static package on a
> > system with binfmt-support, then foreign architecture binaries for all
> > architectures qemu supports will suddenly become executable. This is
> > especially tricky because this will also transparently affect chroot
> > builds with sbuild and neither schroot nor unshare isolation can prevent
> > the emulation from happening. The only ways to stop automatic emulation
> > are uninstalling qemu-user-static on the outside of the build chroot,
> > writing 0 to /proc/sys/fs/binfmt_misc/qemu-$arch or running the build
> > with QEMU_VERSION=1 (unreliable). Since transparent foreign architecture
> > emulation is easily present on a developer's machine and thus
> > influencing the build (even when done inside a chroot) it would be
> > useful to record whether or not foreign architecture binaries can be
> > executed in the buildinfo file.
> 
> Hmm right. To me it feels more like a taint flag though. The
> compilation and execution of the host program feels a bit meh, but
> there's certainly no other way to fetch that information otherwise.
> 
> > I attached a proof-of-concept patch that does exactly that. Since we
> > cannot rely on arch-test being installed in the build environment, this
> > approach cross compiles a small true.c executable for the host
> > architecture. This should always work because gcc is build-essential.
> > The binary outputs a small string instead of just relying on the exit
> > code to guard against QEMU_VERSION=1 "disabling" of emulation. The field
> > 'Can-Execute-Host-Architecture is only added when cross-compiling, i.e
> > when host and build architectures mismatch.
> 
> I'm attaching the slightly revised version with few fixes/changes.

Thanks!

> > >From 62179358b57d09fc8c6bb7a59deb128c67cbe522 Mon Sep 17 00:00:00 2001
> > From: Johannes Schauer Marin Rodrigues 
> > Date: Wed, 18 May 2022 07:11:39 +0200
> > Subject: [PATCH] dpkg-genbuildinfo: when cross-compiling add
> >  Can-Execute-Host-Architecture field
> 
> > +use File::Temp qw(tmpnam);
> 
> This function is marked as obsolete by POSIX, the File::Temp object
> provides a nice interface that can be used instead, perhaps you used
> it but were hit by ETXTBSY errors? (If so closing the descriptor fixes
> the issue, which is what I've done now.) This also means we do not
> need to cleanup the file as the object will do it on its destructor
> when going out of scope.

Yes, I used tmpnam because I got an error that we cannot execute the file as it
was still being written to. While tmpnam might be obsoleted by POSIX, I think
File::Temp uses its own implementation -- at least the docs suggest that it
does. Anyway, this is not really of any importance.

> > +spawn(exec => [ debarch_to_gnutriplet(get_host_arch()) . '-gcc', '-x', 
> > 'c', '-o', $tmpname, '-' ],
> 
> I added honoring the CC envvar, but can potentially result in building
> for the build instead of host arch, as unfortunately we cannot rely on
> an "external build driver" setting a coherent build environment. So
> should probably go back to hardcoding it, but I'm thinking I should
> move all gcc hardcoding into a new vendor-hook that gets the default
> compiler name.

Yes, by ignoring CC I wanted to make sure that a d/rules cannot choose the
wrong compiler.

> > +if ($? == 0 && $stdout eq "ok") {
> > +   $fields->{'Can-Execute-Host-Architecture'} = "true";
> > +} else {
> > +   $fields->{'Can-Execute-Host-Architecture'} = "false";
> 
> I think a taint flag makes more sense. But it has the problem that
> then you might need to check the dpkg version used to see whether
> the check might have been performed, but I'm not sure whether that'd
> be a concern at all? (If its own field would be strongly favored then
> I think it should use the usual "yes"/"no" values used elsewhere.)

I don't have any strong opinion about this. I think I'd be fine with a taint
flag. If any tool wants to read this out in the future, I don't think that
checking the dpkg version is too much to ask for.

Thanks!

cheers, josch

signature.asc
Description: signature


Bug#1011191: dpkg: let buildinfo record whether host architecture binaries can be executed when cross-compiling

2022-05-28 Thread Guillem Jover
Hi!

On Wed, 2022-05-18 at 07:26:02 +0200, Johannes Schauer Marin Rodrigues wrote:
> Package: dpkg
> Version: 1.21.7
> Severity: wishlist
> Tags: patch
> X-Debbugs-Cc: jo...@debian.org

> when cross compiling, one property of the build system that can
> influence the contents of the generate binary packages is whether or not
> the host architecture can be executed. While some platforms can natively
> execute others (like amd64 can execute i386), other combinations are
> more surprising. When installing the qemu-user-static package on a
> system with binfmt-support, then foreign architecture binaries for all
> architectures qemu supports will suddenly become executable. This is
> especially tricky because this will also transparently affect chroot
> builds with sbuild and neither schroot nor unshare isolation can prevent
> the emulation from happening. The only ways to stop automatic emulation
> are uninstalling qemu-user-static on the outside of the build chroot,
> writing 0 to /proc/sys/fs/binfmt_misc/qemu-$arch or running the build
> with QEMU_VERSION=1 (unreliable). Since transparent foreign architecture
> emulation is easily present on a developer's machine and thus
> influencing the build (even when done inside a chroot) it would be
> useful to record whether or not foreign architecture binaries can be
> executed in the buildinfo file.

Hmm right. To me it feels more like a taint flag though. The
compilation and execution of the host program feels a bit meh, but
there's certainly no other way to fetch that information otherwise.

> I attached a proof-of-concept patch that does exactly that. Since we
> cannot rely on arch-test being installed in the build environment, this
> approach cross compiles a small true.c executable for the host
> architecture. This should always work because gcc is build-essential.
> The binary outputs a small string instead of just relying on the exit
> code to guard against QEMU_VERSION=1 "disabling" of emulation. The field
> 'Can-Execute-Host-Architecture is only added when cross-compiling, i.e
> when host and build architectures mismatch.

I'm attaching the slightly revised version with few fixes/changes.

> >From 62179358b57d09fc8c6bb7a59deb128c67cbe522 Mon Sep 17 00:00:00 2001
> From: Johannes Schauer Marin Rodrigues 
> Date: Wed, 18 May 2022 07:11:39 +0200
> Subject: [PATCH] dpkg-genbuildinfo: when cross-compiling add
>  Can-Execute-Host-Architecture field

> +use File::Temp qw(tmpnam);

This function is marked as obsolete by POSIX, the File::Temp object
provides a nice interface that can be used instead, perhaps you used
it but were hit by ETXTBSY errors? (If so closing the descriptor fixes
the issue, which is what I've done now.) This also means we do not
need to cleanup the file as the object will do it on its destructor
when going out of scope.

> +spawn(exec => [ debarch_to_gnutriplet(get_host_arch()) . '-gcc', '-x', 
> 'c', '-o', $tmpname, '-' ],

I added honoring the CC envvar, but can potentially result in building
for the build instead of host arch, as unfortunately we cannot rely on
an "external build driver" setting a coherent build environment. So
should probably go back to hardcoding it, but I'm thinking I should
move all gcc hardcoding into a new vendor-hook that gets the default
compiler name.

> +if ($? == 0 && $stdout eq "ok") {
> +   $fields->{'Can-Execute-Host-Architecture'} = "true";
> +} else {
> +   $fields->{'Can-Execute-Host-Architecture'} = "false";

I think a taint flag makes more sense. But it has the problem that
then you might need to check the dpkg version used to see whether
the check might have been performed, but I'm not sure whether that'd
be a concern at all? (If its own field would be strongly favored then
I think it should use the usual "yes"/"no" values used elsewhere.)

Thanks,
Guillem
From cd5f2c47f8aa60e19a7906d3e38b6e53b899a51d Mon Sep 17 00:00:00 2001
From: Johannes Schauer Marin Rodrigues 
Date: Fri, 27 May 2022 01:33:19 +0200
Subject: [PATCH] dpkg-genbuildinfo: Add new can-execute-cross-built-programs
 tainted flag

[guil...@debian.org:
 - Use File::Temp instead of tmpnam() and push_exit_handler().
 - Set a taint flag instead of a new field.
 - Refactor into a function.
 - Honor CC environment variable.
 - Style fixes. ]

Closes: #1011191
---
 scripts/dpkg-genbuildinfo.pl | 48 ++--
 1 file changed, 46 insertions(+), 2 deletions(-)

diff --git a/scripts/dpkg-genbuildinfo.pl b/scripts/dpkg-genbuildinfo.pl
index e05fce048..81e4636e5 100755
--- a/scripts/dpkg-genbuildinfo.pl
+++ b/scripts/dpkg-genbuildinfo.pl
@@ -28,13 +28,15 @@ use warnings;
 use List::Util qw(any);
 use Cwd;
 use File::Basename;
+use File::Temp;
 use POSIX qw(:fcntl_h :locale_h strftime);
 
 use Dpkg ();
 use Dpkg::Gettext;
 use Dpkg::Checksums;
 use Dpkg::ErrorHandling;
-use Dpkg::Arch qw(get_build_arch get_host_arch debarch_eq);
+use Dpkg::IPC;
+use Dpkg::Arch qw(get_build_arch get_host_arch debarch_eq 

Bug#1011191: dpkg: let buildinfo record whether host architecture binaries can be executed when cross-compiling

2022-05-17 Thread Johannes Schauer Marin Rodrigues
Package: dpkg
Version: 1.21.7
Severity: wishlist
Tags: patch
X-Debbugs-Cc: jo...@debian.org

Hi,

when cross compiling, one property of the build system that can
influence the contents of the generate binary packages is whether or not
the host architecture can be executed. While some platforms can natively
execute others (like amd64 can execute i386), other combinations are
more surprising. When installing the qemu-user-static package on a
system with binfmt-support, then foreign architecture binaries for all
architectures qemu supports will suddenly become executable. This is
especially tricky because this will also transparently affect chroot
builds with sbuild and neither schroot nor unshare isolation can prevent
the emulation from happening. The only ways to stop automatic emulation
are uninstalling qemu-user-static on the outside of the build chroot,
writing 0 to /proc/sys/fs/binfmt_misc/qemu-$arch or running the build
with QEMU_VERSION=1 (unreliable). Since transparent foreign architecture
emulation is easily present on a developer's machine and thus
influencing the build (even when done inside a chroot) it would be
useful to record whether or not foreign architecture binaries can be
executed in the buildinfo file.

I attached a proof-of-concept patch that does exactly that. Since we
cannot rely on arch-test being installed in the build environment, this
approach cross compiles a small true.c executable for the host
architecture. This should always work because gcc is build-essential.
The binary outputs a small string instead of just relying on the exit
code to guard against QEMU_VERSION=1 "disabling" of emulation. The field
'Can-Execute-Host-Architecture is only added when cross-compiling, i.e
when host and build architectures mismatch.

Thanks!

cheers, josch
>From 62179358b57d09fc8c6bb7a59deb128c67cbe522 Mon Sep 17 00:00:00 2001
From: Johannes Schauer Marin Rodrigues 
Date: Wed, 18 May 2022 07:11:39 +0200
Subject: [PATCH] dpkg-genbuildinfo: when cross-compiling add
 Can-Execute-Host-Architecture field

---
 scripts/dpkg-genbuildinfo.pl | 32 +++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/scripts/dpkg-genbuildinfo.pl b/scripts/dpkg-genbuildinfo.pl
index e05fce048..a296a7314 100755
--- a/scripts/dpkg-genbuildinfo.pl
+++ b/scripts/dpkg-genbuildinfo.pl
@@ -28,13 +28,14 @@ use warnings;
 use List::Util qw(any);
 use Cwd;
 use File::Basename;
+use File::Temp qw(tmpnam);
 use POSIX qw(:fcntl_h :locale_h strftime);
 
 use Dpkg ();
 use Dpkg::Gettext;
 use Dpkg::Checksums;
 use Dpkg::ErrorHandling;
-use Dpkg::Arch qw(get_build_arch get_host_arch debarch_eq);
+use Dpkg::Arch qw(get_build_arch get_host_arch debarch_eq 
debarch_to_gnutriplet);
 use Dpkg::Build::Types;
 use Dpkg::Build::Info qw(get_build_env_allowed);
 use Dpkg::BuildOptions;
@@ -46,6 +47,8 @@ use Dpkg::Control;
 use Dpkg::Changelog::Parse;
 use Dpkg::Deps;
 use Dpkg::Dist::Files;
+use Dpkg::Exit qw(push_exit_handler);
+use Dpkg::IPC;
 use Dpkg::Lock;
 use Dpkg::Version;
 use Dpkg::Vendor qw(get_current_vendor run_vendor_hook);
@@ -455,6 +458,33 @@ $fields->{'Installed-Build-Depends'} = 
collect_installed_builddeps($control);
 
 $fields->{'Environment'} = "\n" . cleansed_environment();
 
+if (get_host_arch() ne $fields->{'Build-Architecture'}) {
+# if we are cross-compiling, record whether it was possible to execute the
+# host architecture by cross-compiling and executing a small host-arch
+# binary
+
+my $tmpname = tmpnam();
+push_exit_handler(sub { unlink($tmpname) });
+my ($stdout, $stderr) = ('', '');
+my $testprog = 'int main(){write(1,"ok",2);return 0;}';
+spawn(exec => [ debarch_to_gnutriplet(get_host_arch()) . '-gcc', '-x', 
'c', '-o', $tmpname, '-' ],
+   wait_child => 1, nocheck => 1,
+   to_string => \$stdout,
+   error_to_string => \$stderr,
+   from_string => \$testprog);
+if ($?) {
+   print { *STDOUT } $stdout;
+   print { *STDERR } $stderr;
+   subprocerr("gcc -x c -");
+}
+spawn(exec => [ $tmpname ], error_to_file => '/dev/null', 'to_string' => 
\$stdout, wait_child => 1, nocheck => 1);
+if ($? == 0 && $stdout eq "ok") {
+   $fields->{'Can-Execute-Host-Architecture'} = "true";
+} else {
+   $fields->{'Can-Execute-Host-Architecture'} = "false";
+}
+}
+
 # Generate the buildinfo filename.
 if ($stdout) {
 # Nothing to do.
-- 
2.35.1