bug#45716: Bug in ‘network-interface-names’ when building guix for aarch64 on x86_64.

bug#45716: Bug in ‘network-interface-names’ when building guix for aarch64 on x86_64.

[Stefan]

Hi!

When building for aarch64 on a x86…64 system, then the ‘network-interface-names’ function produces an error.

Because of this building guix for aarch64 fails during the test with this:

test-name: network-interface-names
location: /tmp/guix-build-guix-1.2.0-8.7624ebb.drv-0/source/tests/syscalls.scm:387
source:
+ (test-assert
+   "network-interface-names"
+   (match (remove
+            (lambda (interface)
+              (string-contains interface ":"))
+            (network-interface-names))
+          (((? string? names) ..1)
+           (lset<=
+             string=?
+             names
+             (all-network-interface-names)))))
actual-value: #f
actual-error:
+ (wrong-type-arg
+   "list-tail"
+   "Wrong type argument in position ~A (expecting ~A): ~S"
+   (1 "pair" ())
+   (()))
result: FAIL


I created a little reproducer for this problem:

stefan@guix ~$ cat test.scm 
(use-modules (guix gexp)
             (gnu packages admin))
(with-imported-modules '((guix build syscalls)
                         (guix build utils))
  (computed-file "insterface-names"
    #~(begin
        (use-modules (guix build syscalls)
                     (guix build utils)
                     (ice-9 popen)
                     (ice-9 rdelim))
        (mkdir-p #$output)
        (chdir #$output)
        (with-output-to-file "interface-names"
          (lambda ()
            (display (string-join (all-network-interface-names) ", "))
            (newline)
            (let* ((port (open-input-pipe #$(file-append inetutils "/bin/ifconfig"))))
              (display (string-join 
                        (let loop ((output '()))
                          (let ((line (read-line port)))
                            (if (eof-object? line)
                              (reverse output)
                              (loop (cons line output)))))
                        "\n"))
              (close-pipe port))
            (display (string-join (network-interface-names) ", "))
            (newline))))))

stefan@guix ~$ guix build --system=aarch64-linux -f test.scm 
Folgende Ableitung wird erstellt:
   /gnu/store/l7sypckaywl3djrhagywwzsb2c3984wf-insterface-names.drv
/gnu/store/l7sypckaywl3djrhagywwzsb2c3984wf-insterface-names.drv wird erstellt …
Backtrace:
           7 (primitive-load "/gnu/store/a37szh4h151v66xyl874qfcbm6x?")
In ice-9/ports.scm:
   463:17  6 (call-with-output-file _ _ #:binary _ #:encoding _)
    474:4  5 (_ _)
In ice-9/eval.scm:
    619:8  4 (_ #(#(#<directory (guile-user) 7ffff6f14f00>)))
    155:9  3 (_ #(#(#<directory (guile-user) 7ffff6f14f00>)))
    159:9  2 (_ #(#(#<directory (guile-user) 7ffff6f14f00>)))
In guix/build/syscalls.scm:
  1476:13  1 (bytevector->string-list _ 40 _)
In unknown file:
           0 (list-tail (108 111 0 0 0 0 0 0 0 0 0 0 0 0 0 0 2 0 0 ?) ?)

ERROR: In procedure list-tail:
In procedure list-tail: Wrong type argument in position 1 (expecting pair): ()
builder for `/gnu/store/l7sypckaywl3djrhagywwzsb2c3984wf-insterface-names.drv' failed with exit code 1
Erstellung von /gnu/store/l7sypckaywl3djrhagywwzsb2c3984wf-insterface-names.drv fehlgeschlagen
Das Erstellungsprotokoll kann unter „/var/log/guix/drvs/l7/sypckaywl3djrhagywwzsb2c3984wf-insterface-names.drv.bz2“ eingesehen werden.
guix build: error: build of `/gnu/store/l7sypckaywl3djrhagywwzsb2c3984wf-insterface-names.drv' failed


When the line containing "(display (string-join (network-interface-names) ", "))" is removed, the output looks as follows:

stefan@guix ~$ cat /gnu/store/q7m7dxp4pn7jrw4f6hkwfsiga5zj43x1-insterface-names/interface-names 
lo
lo        Link encap:Local Loopback
          inet addr:127.0.0.1  Bcast:0.0.0.0  Mask:255.0.0.0
          UP LOOPBACK RUNNING  MTU:65536  Metric:1
          RX packets:0 errors:0 dropped:0 overruns:0 frame:0
          TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:1000
          RX bytes:0  TX bytes:0

I think the problem happens in guix/build/syscalls.scm in ‘bytevector->string-list’, which is used by ‘network-interface-name’, certainly in (drop bytes stride) more bytes than available should be dropped, probably because stride is bigger than len:

(define (bytevector->string-list bv stride len)
  "Return the null-terminated strings found in BV every STRIDE bytes.  Read at
most LEN bytes from BV."
  (let loop ((bytes  (take (bytevector->u8-list bv)
                           (min len (bytevector-length bv))))
             (result '()))
    (match bytes
      (()
       (reverse result))
      (_
=>     (loop (drop bytes stride)
             (cons (bytes->string bytes) result))))))


Bye

Stefan

bug#45716: Bug in ‘network-interface-names’ when building guix for aarch64 on x86_64.

[Stefan]

Hi!

This change makes the reproducer working and prints some more values:

diff --git a/guix/build/syscalls.scm b/guix/build/syscalls.scm
index 85c1c45f81..250c577d33 100644
--- a/guix/build/syscalls.scm
+++ b/guix/build/syscalls.scm
@@ -1473,7 +1473,19 @@ most LEN bytes from BV."
        (()
         (reverse result))
        (_
-       (loop (drop bytes stride)
+       (display "bytes: ")
+       (display (length bytes))
+       (newline)
+       (display "stride: ")
+       (display stride)
+       (newline)
+       (display "ifreq-struct-size: ")
+       (display ifreq-struct-size)
+       (newline)
+       (display "sizeof '*: ")
+       (display (sizeof '*))
+       (newline)
+       (loop (drop bytes (min stride (length bytes)))
              (cons (bytes->string bytes) result))))))
 
  (define* (network-interface-names #:optional sock)


So indeed, the number of bytes is only 32, if build with ‘./pre-inst-env guix build --system=aarch64-linux -f ~/test.scm’:

bytes: 32
stride: 40
sizeof '*: 8

If this is instead build with just ‘./pre-inst-env guix build -f ~/test.scm’, the output is as expected:

bytes: 40
stride: 40
sizeof '*: 8
 
I have however no clue, why the number of bytes is only 32 in case of aarch64. This value is taken from the struct ifconf read via ioctl(SIOCGIFCONF).

Danny, does that maybe ring a bell?

To be precise this is all running in a virtual machine with virtio-net-pci. Both kernel and qemu on the host are rather old. However, building for x86_64 works just fine, so I think the host setup is unrelated.


Bye

Stefan

bug#45716: Bug in ‘network-interface-names’ when building guix for aarch64 on x86_64.

[Danny Milosavljevic]

[-- Attachment #1: Type: text/plain, Size: 1026 bytes --]

Hi Stefan,

>I have however no clue, why the number of bytes is only 32 in case of aarch64. This value is taken from the struct ifconf read via ioctl(SIOCGIFCONF).

guix/build/syscalls.scm has:

>(define ifreq-struct-size
>  ;; 'struct ifreq' begins with an array of IF_NAMESIZE bytes containing the
>  ;; interface name (nul-terminated), followed by a bunch of stuff.  This is
>  ;; its size in bytes.
>  (if (= 8 (sizeof '*))
>      40
>      32))

I think that should rather be (sizeof* '*) instead of (sizeof '*).  Ludo?

Can you try printing (@@ (guix build syscalls) ifreq-struct-size) directly ?

>To be precise this is all running in a virtual machine with virtio-net-pci.
>Both kernel and qemu on the host are rather old. However, building for x86_64
>works just fine, so I think the host setup is unrelated.

Is it qemu-user or qemu-system ?

(Note: Empirically, on armhf, that struct is 32 Bytes big in C.
And indeed the Linux kernel contains a compat struct ifreq32 in fs/compat_ioctl.c)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

bug#45716: Bug in ‘network-interface-names’ when building guix for aarch64 on x86_64.

[Stefan]

Hi Danny and Ludo’!

> Can you try printing (@@ (guix build syscalls) ifreq-struct-size) directly?

I did that, it's printing 40 in both cases. That’s the value which is ending up in stride.

> Is it qemu-user or qemu-system?

~$ /usr/local/bin/qemu-system-x86_64 --version
QEMU emulator version 2.12.1 (-dirty)
Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers

> (Note: Empirically, on armhf, that struct is 32 Bytes big in C.
> And indeed the Linux kernel contains a compat struct ifreq32 in fs/compat_ioctl.c)



Bye

Stefan

bug#45716: Bug in ‘network-interface-names’ when building guix for aarch64 on x86_64.

[Stefan]

Hi!

> I think that should rather be (sizeof* '*) instead of (sizeof '*).  Ludo?

For aarch64 I tried using ‘sizeof*’ instead of ‘sizeof’ in ‘ifreq-struct-size’, but this makes no different at all. Well, pointers have to be 8 bytes long on x86_64 and aarch64, so this was expected. However, it should make a difference for armhf, but I didn't try that.


Bye

Stefan

bug#45716: Bug in ‘network-interface-names’ when building guix for aarch64 on x86_64.

[Stefan]

Hi!

I think I found the root cause of this issue in qemu. I'm about to prepare a patch.


Bye

Stefan

bug#45716: gnu: qemu: Fix ioclt(…, SIOCGIFCONF, …) for emulated 64 bit architectures.

[Stefan]

* gnu/packages/virtualization.scm (qemu): Add a snippet to fix a bug in the
do_ioctl_ifconf() function of qemu to make ioclt(…, SIOCGIFCONF, …) work for
emulated 64 bit architectures.

The sizeof(struct ifreq) is 40 for 64 bit and 32 for 32 bit architectures.
This structure contains a union of other structures, of which struct ifmap
is the biggest for 64 bit architectures. Calling ioclt(…, SIOCGIFCONF, …)
fills a struct sockaddr of that union, and do_ioctl_ifconf() only considered
that struct sockaddr for the size of the union, which has the same size as
struct ifmap on 32 bit architectures. So do_ioctl_ifconf() assumed a wrong
size of 32 for struct ifreq instead of the correct size of 40 on 64 bit
architectures.

The fix makes do_ioctl_ifconf() handle struct ifmap as the biggest part of
the union, treating struct ifreq with the correct size.

This fixes (@ (guix build syscalls) network-interface-names) when used in
emulated 64 bit architectures.
---
 gnu/packages/virtualization.scm | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/virtualization.scm b/gnu/packages/virtualization.scm
index 10eae8ea7d..f82a19ecf3 100644
--- a/gnu/packages/virtualization.scm
+++ b/gnu/packages/virtualization.scm
@@ -140,7 +140,17 @@
                (sha256
                 (base32
                  "1rd41wwlvp0vpialjp2czs6i3lsc338xc72l3zkbb7ixjfslw5y9"))
-              (patches (search-patches "qemu-build-info-manual.patch"))))
+              (patches (search-patches "qemu-build-info-manual.patch"))
+              (modules '((guix build utils)))
+              (snippet
+               '(begin
+                  (substitute* '("linux-user/syscall.c")
+                    (("^([[:blank:]]*)const argtype ifreq_arg_type.*$" line indent)
+                     (string-append line indent
+                                    "const argtype ifreq_max_type[] = { MK_STRUCT(STRUCT_ifmap_ifreq) };\n"))
+                    (("^([[:blank:]]*)target_ifreq_size[[:blank:]]=.*$" _ indent)
+                     (string-append indent "target_ifreq_size = thunk_type_size(ifreq_max_type, 0);")))
+                  #t))))
      (outputs '("out" "doc"))            ;4.7 MiB of HTML docs
      (build-system gnu-build-system)
      (arguments
-- 
2.29.2

bug#45716: gnu: qemu: Fix ioclt(…, SIOCGIFCONF, …) for emulated 64 bit architectures.

[Ludovic Courtès]

Hi Stefan,

Stefan <stefan-guix@vodafonemail.de> skribis:

> * gnu/packages/virtualization.scm (qemu): Add a snippet to fix a bug in the
> do_ioctl_ifconf() function of qemu to make ioclt(…, SIOCGIFCONF, …) work for
> emulated 64 bit architectures.
>
> The sizeof(struct ifreq) is 40 for 64 bit and 32 for 32 bit architectures.
> This structure contains a union of other structures, of which struct ifmap
> is the biggest for 64 bit architectures. Calling ioclt(…, SIOCGIFCONF, …)
> fills a struct sockaddr of that union, and do_ioctl_ifconf() only considered
> that struct sockaddr for the size of the union, which has the same size as
> struct ifmap on 32 bit architectures. So do_ioctl_ifconf() assumed a wrong
> size of 32 for struct ifreq instead of the correct size of 40 on 64 bit
> architectures.
>
> The fix makes do_ioctl_ifconf() handle struct ifmap as the biggest part of
> the union, treating struct ifreq with the correct size.
>
> This fixes (@ (guix build syscalls) network-interface-names) when used in
> emulated 64 bit architectures.

Woow, good catch!

> +              (modules '((guix build utils)))
> +              (snippet
> +               '(begin
> +                  (substitute* '("linux-user/syscall.c")
> +                    (("^([[:blank:]]*)const argtype ifreq_arg_type.*$" line indent)
> +                     (string-append line indent
> +                                    "const argtype ifreq_max_type[] = { MK_STRUCT(STRUCT_ifmap_ifreq) };\n"))
> +                    (("^([[:blank:]]*)target_ifreq_size[[:blank:]]=.*$" _ indent)
> +                     (string-append indent "target_ifreq_size = thunk_type_size(ifreq_max_type, 0);")))
> +                  #t))))

Could you move the explanation as a comment above the ‘substitute*’
form, and/or link to the upstream bug report/discussion/submission?

Thank you!

Ludo’.

bug#45716: [PATCH 1/1] gnu: qemu: Fix ioclt(…, SIOCGIFCONF, …) for emulated 64 bit architectures.

[Stefan]

* gnu/packages/virtualization.scm (qemu): Add a snippet to fix a bug in the
do_ioctl_ifconf() function of qemu to make ioclt(…, SIOCGIFCONF, …) work for
emulated 64 bit architectures.

The sizeof(struct ifreq) is 40 for 64 bit and 32 for 32 bit architectures.
This structure contains a union of other structures, of which struct ifmap
is the biggest for 64 bit architectures. Calling ioclt(…, SIOCGIFCONF, …)
fills a struct sockaddr of that union, and do_ioctl_ifconf() only considered
that struct sockaddr for the size of the union, which has the same size as
struct ifmap on 32 bit architectures. So do_ioctl_ifconf() assumed a wrong
size of 32 for struct ifreq instead of the correct size of 40 on 64 bit
architectures.

The fix makes do_ioctl_ifconf() handle struct ifmap as the biggest part of
the union, treating struct ifreq with the correct size.

This fixes (@ (guix build syscalls) network-interface-names) when used in
emulated 64 bit architectures.
---
  gnu/packages/virtualization.scm | 17 ++++++++++++++++-
  1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/virtualization.scm b/gnu/packages/virtualization.scm
index 10eae8ea7d..e773fe357d 100644
--- a/gnu/packages/virtualization.scm
+++ b/gnu/packages/virtualization.scm
@@ -140,7 +140,22 @@
                (sha256
                 (base32
                  "1rd41wwlvp0vpialjp2czs6i3lsc338xc72l3zkbb7ixjfslw5y9"))
-              (patches (search-patches "qemu-build-info-manual.patch"))))
+              (patches (search-patches "qemu-build-info-manual.patch"))
+              (modules '((guix build utils)))
+              (snippet
+               '(begin
+                  ;; Fix a bug in the do_ioctl_ifconf() function of qemu to
+                  ;; make ioclt(…, SIOCGIFCONF, …) work for emulated 64 bit
+                  ;; architectures.  The size of struct ifreq is handled
+                  ;; incorrectly.
+                  ;; https://lists.nongnu.org/archive/html/qemu-devel/2021-01/msg01545.html
+                  (substitute* '("linux-user/syscall.c")
+                    (("^([[:blank:]]*)const argtype ifreq_arg_type.*$" line indent)
+                     (string-append line indent
+                                    "const argtype ifreq_max_type[] = { MK_STRUCT(STRUCT_ifmap_ifreq) };\n"))
+                    (("^([[:blank:]]*)target_ifreq_size[[:blank:]]=.*$" _ indent)
+                     (string-append indent "target_ifreq_size = thunk_type_size(ifreq_max_type, 0);")))
+                  #t))))
      (outputs '("out" "doc"))            ;4.7 MiB of HTML docs
      (build-system gnu-build-system)
      (arguments
-- 
2.29.2

bug#45716: [PATCH 1/1] gnu: qemu: Fix ioclt(…, SIOCGIFCONF, …) for emulated 64 bit architectures.

[Ludovic Courtès]

Hi,

Stefan <stefan-guix@vodafonemail.de> skribis:

> * gnu/packages/virtualization.scm (qemu): Add a snippet to fix a bug in the
> do_ioctl_ifconf() function of qemu to make ioclt(…, SIOCGIFCONF, …) work for
> emulated 64 bit architectures.
>
> The sizeof(struct ifreq) is 40 for 64 bit and 32 for 32 bit architectures.
> This structure contains a union of other structures, of which struct ifmap
> is the biggest for 64 bit architectures. Calling ioclt(…, SIOCGIFCONF, …)
> fills a struct sockaddr of that union, and do_ioctl_ifconf() only considered
> that struct sockaddr for the size of the union, which has the same size as
> struct ifmap on 32 bit architectures. So do_ioctl_ifconf() assumed a wrong
> size of 32 for struct ifreq instead of the correct size of 40 on 64 bit
> architectures.
>
> The fix makes do_ioctl_ifconf() handle struct ifmap as the biggest part of
> the union, treating struct ifreq with the correct size.
>
> This fixes (@ (guix build syscalls) network-interface-names) when used in
> emulated 64 bit architectures.

Applied, thanks!

Ludo’.

bug#45716: Bug in ‘network-interface-names’ when building guix for aarch64 on x86_64.

[Stefan]

Hi Ludo’!

> Applied, thanks!

Thanks.

Unfortunately the bug still appears on ci.guix.gnu.org, for example see <https://ci.guix.gnu.org/build/204877/details>.

I think that the build machines need to be reconfigured to get the fixed qemu version.


Bye

Stefan

bug#45716: Bug in ‘network-interface-names’ when building guix for aarch64 on x86_64.

[Leo Famulari]

On Sat, Jan 23, 2021 at 04:27:05PM +0100, Stefan wrote:
> Hi Ludo’!
> 
> > Applied, thanks!
> 
> Thanks.
> 
> Unfortunately the bug still appears on ci.guix.gnu.org, for example see <https://ci.guix.gnu.org/build/204877/details>.
> 
> I think that the build machines need to be reconfigured to get the fixed qemu version.

Forwarding to guix-sysadmin so it doesn't get lost.

bug#45716: Bug in ‘network-interface-names’ when building guix for aarch64 on x86_64.

[Ludovic Courtès]

Hi,

Leo Famulari <leo@famulari.name> skribis:

> On Sat, Jan 23, 2021 at 04:27:05PM +0100, Stefan wrote:
>> Hi Ludo’!
>> 
>> > Applied, thanks!
>> 
>> Thanks.
>> 
>> Unfortunately the bug still appears on ci.guix.gnu.org, for example see <https://ci.guix.gnu.org/build/204877/details>.
>> 
>> I think that the build machines need to be reconfigured to get the fixed qemu version.
>
> Forwarding to guix-sysadmin so it doesn't get lost.

Yes, I noticed that this test is still failing on emulated AArch64.

Mathieu, did you reconfigure the build nodes recently or should we do so?

Thanks,
Ludo’.

bug#45716: Bug in ‘network-interface-names’ when building guix for aarch64 on x86_64.

[Mathieu Othacehe]

Hey Ludo,

> Mathieu, did you reconfigure the build nodes recently or should we do so?

Yes, I reconfigured them yesterday with Guix at
85843123dedc3060a128457f7814d7461cccb412.

Mathieu

bug#45716: Bug in ‘network-interface-names’ when building guix for aarch64 on x86_64.

[Stefan]

[-- Attachment #1: Type: text/plain, Size: 239 bytes --]

Hi Mathieu!

This is still failing, see <https://ci.guix.gnu.org/build/233248/details>.

> Yes, I reconfigured them yesterday


Could it be that the services qemu-binfmt and guix-daemon still need to be restarted?


Bye

Stefan

[-- Attachment #2: Type: text/html, Size: 594 bytes --]

bug#45716: Bug in ‘network-interface-names’ when building guix for aarch64 on x86_64.

[Mathieu Othacehe]

Hey Stefan,

> Could it be that the services qemu-binfmt and guix-daemon still need to be restarted?

Just restarted those services, let's see what happens!

Thanks,

Mathieu

bug#45716: Bug in ‘network-interface-names’ when building guix for aarch64 on x86_64.

[Stefan]

[-- Attachment #1: Type: text/plain, Size: 271 bytes --]

Hi Mathieu!

> Just restarted those services, let's see what happens!


Thanks! Now it is working: <https://ci.guix.gnu.org/build/283711/details>

I just can’t tell if it got build on a x86_64 or an aarch64 machine. But the result matters. :-)


Bye

Stefan

[-- Attachment #2: Type: text/html, Size: 655 bytes --]

bug#45716: Bug in ‘network-interface-names’ when building guix for aarch64 on x86_64.

[Ludovic Courtès]

Hi Danny,

Danny Milosavljevic <dannym@scratchpost.org> skribis:

>>(define ifreq-struct-size
>>  ;; 'struct ifreq' begins with an array of IF_NAMESIZE bytes containing the
>>  ;; interface name (nul-terminated), followed by a bunch of stuff.  This is
>>  ;; its size in bytes.
>>  (if (= 8 (sizeof '*))
>>      40
>>      32))
>
> I think that should rather be (sizeof* '*) instead of (sizeof '*).  Ludo?

As you found out, it makes no difference.  What changes is that
‘sizeof*’ is a compile-time computation (but it honors cross-compilation
targets) whereas ‘sizeof’ is a call made at run time.

Ludo’.