bug#74669: [PATCH v3] gnu: elfutils: Fix cross-compilation.

[Zheng Junjie <zhengjunjie@iscas.ac.cn>]

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

Zheng Junjie <zhengjunjie@iscas.ac.cn> writes:

> * gnu/packages/elf.scm (elfutils): Fix cross-compilation
> [arguments]: When cross-compilation, apply patch.
> <#:phases>: Use G-expressions.
> * gnu/packages/patches/elfutils-libdwfl-string-overflow.patch: New file.
> * gnu/local.mk (dist_patch_DATA): Register it.
>
> Change-Id: Ia1ab9c09124a140608bda34bf62f0ae80eb44f9b
> ---
>  gnu/local.mk                                  |   1 +
>  gnu/packages/elf.scm                          |  94 +++++++-------
>  .../elfutils-libdwfl-string-overflow.patch    | 116 ++++++++++++++++++
>  3 files changed, 169 insertions(+), 42 deletions(-)
>  create mode 100644 gnu/packages/patches/elfutils-libdwfl-string-overflow.patch
>
> diff --git a/gnu/local.mk b/gnu/local.mk
> index 39b142af5a..67d0085b71 100644
> --- a/gnu/local.mk
> +++ b/gnu/local.mk
> @@ -1177,6 +1177,7 @@ dist_patch_DATA =						\
>    %D%/packages/patches/eigen-fix-strict-aliasing-bug.patch	\
>    %D%/packages/patches/einstein-build.patch			\
>    %D%/packages/patches/elfutils-tests-ptrace.patch		\
> +  %D%/packages/patches/elfutils-libdwfl-string-overflow.patch	\
>    %D%/packages/patches/elixir-path-length.patch			\
>    %D%/packages/patches/elm-ghc9.2.patch	\
>    %D%/packages/patches/elm-offline-package-registry.patch	\
> diff --git a/gnu/packages/elf.scm b/gnu/packages/elf.scm
> index 81a753a483..8f036ab221 100644
> --- a/gnu/packages/elf.scm
> +++ b/gnu/packages/elf.scm
> @@ -11,6 +11,7 @@
>  ;;; Copyright © 2021 Leo Le Bouter <lle-bout@zaclys.net>
>  ;;; Copyright © 2021 Maxime Devos <maximedevos@telenet.be>
>  ;;; Copyright © 2023, 2024 Janneke Nieuwenhuizen <janneke@gnu.org>
> +;;; Copyright © 2024 Zheng Junjie <873216071@qq.com>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -99,48 +100,57 @@ (define-public elfutils
>               '())
>  
>         #:phases
> -       (modify-phases %standard-phases
> -         ;; No reason has been found for this test to reliably fail on aarch64-linux.
> -         (add-after 'unpack 'disable-failing-aarch64-tests
> -           (lambda _
> -             (substitute* "tests/Makefile.in"
> -               (("run-backtrace-native.sh") ""))
> -             #t))
> -         ,@(if (target-riscv64?)
> -             `((add-after 'unpack 'disable-failing-riscv64-test
> -                 (lambda _
> -                   (substitute* "tests/Makefile.in"
> -                     ;; dwfl_thread_getframes: No DWARF information found
> -                     (("run-backtrace-dwarf.sh") "")
> -                     ;; These tests have several errors:
> -                     ;; unknown program header entry type 0x70000003
> -                     ;; '.riscv.attributes' has unsupported type 1879048195
> -                     (("run-reverse-sections-self.sh") "")
> -                     (("run-strip-strmerge.sh") "")
> -                     (("run-elflint-self.sh") "")))))
> -             '())
> -         ,@(if (system-hurd?)
> -             `((add-after 'unpack 'skip-tests
> -                 (lambda _
> -                   (substitute* '("tests/elfstrtab.c"
> -                                  "tests/emptyfile.c")
> -                     (("elf_version \\(EV_CURRENT\\);" all)
> -                      "exit (77);"))
> -                   (substitute* '("tests/run-all-dwarf-ranges.sh"
> -                                  "tests/run-allfcts-multi.sh"
> -                                  "tests/run-attr-integrate-skel.sh"
> -                                  "tests/run-bug1-test.sh"
> -                                  "tests/run-copyadd-sections.sh"
> -                                  "tests/run-deleted.sh"
> -                                  "tests/run-get-units-split.sh"
> -                                  "tests/run-native-test.sh"
> -                                  "tests/run-readelf-loc.sh"
> -                                  "tests/run-readelf-ranges.sh"
> -                                  "tests/run-unit-info.sh"
> -                                  "tests/run-varlocs.sh")
> -                               (("^#!.*" all)
> -                                (string-append all "exit 77;\n"))))))
> -             '()))))
> +       ,#~(modify-phases %standard-phases
> +            ;; No reason has been found for this test to reliably fail on aarch64-linux.
> +            (add-after 'unpack 'disable-failing-aarch64-tests
> +              (lambda _
> +                (substitute* "tests/Makefile.in"
> +                  (("run-backtrace-native.sh") ""))
> +                #t))
> +            #$@(if (target-riscv64?)
> +                   #~((add-after 'unpack 'disable-failing-riscv64-test
> +                        (lambda _
> +                          (substitute* "tests/Makefile.in"
> +                            ;; dwfl_thread_getframes: No DWARF information found
> +                            (("run-backtrace-dwarf.sh") "")
> +                            ;; These tests have several errors:
> +                            ;; unknown program header entry type 0x70000003
> +                            ;; '.riscv.attributes' has unsupported type 1879048195
> +                            (("run-reverse-sections-self.sh") "")
> +                            (("run-strip-strmerge.sh") "")
> +                            (("run-elflint-self.sh") "")))))
> +                   #~())
> +            #$@(if (system-hurd?)
> +                   #~((add-after 'unpack 'skip-tests
> +                        (lambda _
> +                          (substitute* '("tests/elfstrtab.c"
> +                                         "tests/emptyfile.c")
> +                            (("elf_version \\(EV_CURRENT\\);" all)
> +                             "exit (77);"))
> +                          (substitute* '("tests/run-all-dwarf-ranges.sh"
> +                                         "tests/run-allfcts-multi.sh"
> +                                         "tests/run-attr-integrate-skel.sh"
> +                                         "tests/run-bug1-test.sh"
> +                                         "tests/run-copyadd-sections.sh"
> +                                         "tests/run-deleted.sh"
> +                                         "tests/run-get-units-split.sh"
> +                                         "tests/run-native-test.sh"
> +                                         "tests/run-readelf-loc.sh"
> +                                         "tests/run-readelf-ranges.sh"
> +                                         "tests/run-unit-info.sh"
> +                                         "tests/run-varlocs.sh")
> +                            (("^#!.*" all)
> +                             (string-append all "exit 77;\n"))))))
> +                   #~())
> +            #$@(if (%current-target-system)
> +                   #~((add-after 'unpack 'patch
> +                        (lambda* (#:key native-inputs #:allow-other-keys)
> +                          (invoke
> +                           "patch" "-p1" "--force" "-i"
> +                           #$(local-file
> +                              (search-patch
> +                               "elfutils-libdwfl-string-overflow.patch"))))))
> +                   #~()))))
>  
>      (native-inputs (list m4))
>      (inputs (list xz zlib))
> diff --git a/gnu/packages/patches/elfutils-libdwfl-string-overflow.patch b/gnu/packages/patches/elfutils-libdwfl-string-overflow.patch
> new file mode 100644
> index 0000000000..e9d51359c4
> --- /dev/null
> +++ b/gnu/packages/patches/elfutils-libdwfl-string-overflow.patch
> @@ -0,0 +1,116 @@
> +from https://sourceware.org/cgit/elfutils/patch/?id=0873ae782d14e672e8344775e76b7fca0a8b41bf
> +
> +Adjust the changelog so it can be applied on elfutils 0.187.
> +
> +From 0873ae782d14e672e8344775e76b7fca0a8b41bf Mon Sep 17 00:00:00 2001
> +From: Mark Wielaard <mark@klomp.org>
> +Date: Thu, 28 Jul 2022 15:31:12 +0200
> +Subject: libdwfl: Rewrite reading of ar_size in elf_begin_rand
> +MIME-Version: 1.0
> +Content-Type: text/plain; charset=UTF-8
> +Content-Transfer-Encoding: 8bit
> +
> +With GCC 12.1.1, glibc 2.35, -fsanitize=undefined and
> +-D_FORTIFY_SOURCE=3 we get the following error message:
> +
> +In file included from /usr/include/ar.h:22,
> +                 from ../libelf/libelfP.h:33,
> +                 from core-file.c:31:
> +In function ‘pread’,
> +    inlined from ‘pread_retry’ at ../lib/system.h:188:21,
> +    inlined from ‘elf_begin_rand’ at core-file.c:86:16,
> +    inlined from ‘core_file_read_eagerly’ at core-file.c:205:15:
> +/usr/include/bits/unistd.h:74:10: error: ‘__pread_alias’ writing 58 or more bytes into a region of size 10 overflows the destination [-Werror=stringop-overflow=]
> +   74 |   return __glibc_fortify (pread, __nbytes, sizeof (char),
> +      |          ^~~~~~~~~~~~~~~
> +/usr/include/ar.h: In function ‘core_file_read_eagerly’:
> +/usr/include/ar.h:41:10: note: destination object ‘ar_size’ of size 10
> +   41 |     char ar_size[10];           /* File size, in ASCII decimal.  */
> +      |          ^~~~~~~
> +/usr/include/bits/unistd.h:50:16: note: in a call to function ‘__pread_alias’ declared with attribute ‘access (write_only, 2, 3)’
> +   50 | extern ssize_t __REDIRECT (__pread_alias,
> +      |                ^~~~~~~~~~
> +cc1: all warnings being treated as errors
> +
> +The warning disappears when dropping either -fsanitize=undefined
> +or when using -D_FORTIFY_SOURCE=2. It looks like a false positive.
> +But I haven't figured out how/why it happens.
> +
> +The code is a little tricky to proof correct though. The ar_size
> +field is a not-zero terminated string ASCII decimal, right-padded
> +with spaces. Which is then converted with strtoll. Relying on the
> +fact that the struct ar_hdr is zero initialized, so there will be
> +a zero byte after the ar_size field.
> +
> +Rewrite the code to just use a zero byte terminated char array.
> +Which is much easier to reason about. As a bonus the error disappears.
> +
> +Signed-off-by: Mark Wielaard <mark@klomp.org>
> +---
> + libdwfl/ChangeLog   |  5 +++++
> + libdwfl/core-file.c | 26 ++++++++++++++++----------
> + 2 files changed, 21 insertions(+), 10 deletions(-)
> +
> +diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
> +index 75c53948d..acdaa0138 100644
> +--- a/libdwfl/ChangeLog
> ++++ b/libdwfl/ChangeLog
> +@@ -1,0 +1,5 @@
> ++2022-07-28  Mark Wielaard  <mark@klomp.org>
> ++
> ++	* core-file.c (elf_begin_rand): Replace struct ar_hdr h with
> ++	a char ar_size[AR_SIZE_CHARS + 1] array to read size.
> ++
> +2022-04-22  Mark Wielaard  <mark@klomp.org>
> +
> +	* debuginfod-client.c (init_control): New static pthread_once_t.
> +diff --git a/libdwfl/core-file.c b/libdwfl/core-file.c
> +index cefc3db0f..4418ef338 100644
> +--- a/libdwfl/core-file.c
> ++++ b/libdwfl/core-file.c
> +@@ -75,26 +75,32 @@ elf_begin_rand (Elf *parent, off_t offset, off_t size, off_t *next)
> +      from the archive header to override SIZE.  */
> +   if (parent->kind == ELF_K_AR)
> +     {
> +-      struct ar_hdr h = { .ar_size = "" };
> +-
> +-      if (unlikely (parent->maximum_size - offset < sizeof h))
> ++      /* File size, in ASCII decimal, right-padded with ASCII spaces.
> ++         Max 10 characters. Not zero terminated. So make this ar_size
> ++         array one larger and explicitly zero terminate it.  As needed
> ++         for strtoll.  */
> ++      #define AR_SIZE_CHARS 10
> ++      char ar_size[AR_SIZE_CHARS + 1];
> ++      ar_size[AR_SIZE_CHARS] = '\0';
> ++
> ++      if (unlikely (parent->maximum_size - offset < sizeof (struct ar_hdr)))
> + 	return fail (ELF_E_RANGE);
> + 
> +       if (parent->map_address != NULL)
> +-	memcpy (h.ar_size, parent->map_address + parent->start_offset + offset,
> +-		sizeof h.ar_size);
> ++	memcpy (ar_size, parent->map_address + parent->start_offset + offset,
> ++		AR_SIZE_CHARS);
> +       else if (unlikely (pread_retry (parent->fildes,
> +-				      h.ar_size, sizeof (h.ar_size),
> ++				      ar_size, AR_SIZE_CHARS,
> + 				      parent->start_offset + offset
> + 				      + offsetof (struct ar_hdr, ar_size))
> +-			 != sizeof (h.ar_size)))
> ++			 != AR_SIZE_CHARS))
> + 	return fail (ELF_E_READ_ERROR);
> + 
> +-      offset += sizeof h;
> ++      offset += sizeof (struct ar_hdr);
> + 
> +       char *endp;
> +-      size = strtoll (h.ar_size, &endp, 10);
> +-      if (unlikely (endp == h.ar_size)
> ++      size = strtoll (ar_size, &endp, 10);
> ++      if (unlikely (endp == ar_size)
> + 	  || unlikely ((off_t) parent->maximum_size - offset < size))
> + 	return fail (ELF_E_INVALID_ARCHIVE);
> +     }
> +-- 
> +cgit 
> +
>
> base-commit: 0787a180b3ca64d3958438920ca1b58a2a27fba4

pushed, closing.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]