Bug#1035398: [pre-approval] unblock: dwarves/1.24-4.1
Control: tags -1 confirmed moreinfo
On 2023-05-02 20:59:26 +0200, Salvatore Bonaccorso wrote:
> Package: release.debian.org
> Severity: normal
> User: release.debian.org@packages.debian.org
> Usertags: unblock
> X-Debbugs-Cc: dwarves@packages.debian.org, Aurelien Jarno <aurel32@debian.org>, kibi@debian.org, vagrant@debian.org, Domenico Andreoli <cavok@debian.org>, carnil@debian.org
> Control: affects -1 + src:dwarves
>
> Dear release team,
>
> Please unblock package dwarves
>
> [ Reason ]
> Back in #1033301, Aurelien reported that the arm64 kernel size did
> increase significantly due to issues with BTF deduplication. First
> suspected to be a Linux kernel upstream issue, Aurelien discussed this
> on with upstream and it was found that the issue is caused by a
> src:dwarves regression (applied in 1.24-4).
>
> Details in https://bugs.debian.org/1033301#31
>
> The (not yet uploaded) dwarves upload with attache debdiff
> cherry-picks the upstream commit.
Please go ahead and remove the moreinfo tag once the package is
available in unstable.
Cheers
>
> (Please provide enough (but not too much) information to help
> the release team to judge the request efficiently. E.g. by
> filling in the sections below.)
>
> [ Impact ]
> Increased arm64 kernel size.
>
> [ Tests ]
> Apart from the report from Aurelien[1], package passes its autopkgtest.
>
> [1] https://lore.kernel.org/linux-arm-kernel/ZEzHaJUP21Ln5XBt@aurel32.net/
> [ Risks ]
> The upstream commit zero-initializes memory which previous was not
> initialized after allocation, and might have contained garbage values
> which were used. The fix is isolated as a oneliner.
>
> [ Checklist ]
> [x] all changes are documented in the d/changelog
> [x] I reviewed all changes and I approve them
> [x] attach debdiff against the package in testing
>
> [ Other info ]
> Ideally this enters the archive before a next upload for src:linux is
> built (and which would be aimed for bookworm).
>
> unblock dwarves/1.24-4.1
>
> Regards,
> Salvatore
> diff -Nru dwarves-1.24/debian/changelog dwarves-1.24/debian/changelog
> --- dwarves-1.24/debian/changelog 2022-12-10 10:11:28.000000000 +0100
> +++ dwarves-1.24/debian/changelog 2023-05-02 20:37:16.000000000 +0200
> @@ -1,3 +1,13 @@
> +dwarves (1.24-4.1) unstable; urgency=medium
> +
> + * Non-maintainer upload.
> + * dwarves: Zero-initialize struct cu in cu__new() to prevent incorrect BTF
> + types.
> + Fixes BTF deduplication issues causing arm64 kernel size increase.
> + Thanks to Aurelien Jarno (Closes: #1033301)
> +
> + -- Salvatore Bonaccorso <carnil@debian.org> Tue, 02 May 2023 20:37:16 +0200
> +
> dwarves (1.24-4) unstable; urgency=medium
>
> * Backport upstream patches to support newer toolchains.
> diff -Nru dwarves-1.24/debian/patches/03-dwarves-Zero-initialize-struct-cu-in-cu__new-to-prev.patch dwarves-1.24/debian/patches/03-dwarves-Zero-initialize-struct-cu-in-cu__new-to-prev.patch
> --- dwarves-1.24/debian/patches/03-dwarves-Zero-initialize-struct-cu-in-cu__new-to-prev.patch 1970-01-01 01:00:00.000000000 +0100
> +++ dwarves-1.24/debian/patches/03-dwarves-Zero-initialize-struct-cu-in-cu__new-to-prev.patch 2023-05-02 20:37:16.000000000 +0200
> @@ -0,0 +1,94 @@
> +From: Alan Maguire <alan.maguire@oracle.com>
> +Date: Fri, 21 Oct 2022 16:02:03 +0100
> +Subject: dwarves: Zero-initialize struct cu in cu__new() to prevent incorrect
> + BTF types
> +Origin: https://git.kernel.org/pub/scm/devel/pahole/pahole.git/commit?id=b72f5188856df0abf45e1a707856bb4e4e86153c
> +Bug-Debian: https://bugs.debian.org/1033301
> +
> +BTF deduplication was throwing some strange results, where core kernel
> +data types were failing to deduplicate due to the return values
> +of function type members being void (0) instead of the actual type
> +(unsigned int). An example of this can be seen below, where
> +"struct dst_ops" was failing to deduplicate between kernel and
> +module:
> +
> +struct dst_ops {
> + short unsigned int family;
> + unsigned int gc_thresh;
> + int (*gc)(struct dst_ops *);
> + struct dst_entry * (*check)(struct dst_entry *, __u32);
> + unsigned int (*default_advmss)(const struct dst_entry *);
> + unsigned int (*mtu)(const struct dst_entry *);
> +...
> +
> +struct dst_ops___2 {
> + short unsigned int family;
> + unsigned int gc_thresh;
> + int (*gc)(struct dst_ops___2 *);
> + struct dst_entry___2 * (*check)(struct dst_entry___2 *, __u32);
> + void (*default_advmss)(const struct dst_entry___2 *);
> + void (*mtu)(const struct dst_entry___2 *);
> +...
> +
> +This was seen with
> +
> +bcc648a10cbc ("btf_encoder: Encode DW_TAG_unspecified_type returning routines as void")
> +
> +...which rewrites the return value as 0 (void) when it is marked
> +as matching DW_TAG_unspecified_type:
> +
> +static int32_t btf_encoder__tag_type(struct btf_encoder *encoder, uint32_t type_id_off, uint32_t tag_type)
> +{
> + if (tag_type == 0)
> + return 0;
> +
> + if (encoder->cu->unspecified_type.tag && tag_type == encoder->cu->unspecified_type.type) {
> + // No provision for encoding this, turn it into void.
> + return 0;
> + }
> +
> + return type_id_off + tag_type;
> +}
> +
> +However the odd thing was that on further examination, the unspecified type
> +was not being set, so why was this logic being tripped? Futher debugging
> +showed that the encoder->cu->unspecified_type.tag value was garbage, and
> +the type id happened to collide with "unsigned int"; as a result we
> +were replacing unsigned ints with void return values, and since this
> +was being done to function type members in structs, it triggered a
> +type mismatch which failed deduplication between kernel and module.
> +
> +The fix is simply to calloc() the cu in cu__new() instead.
> +
> +Committer notes:
> +
> +We have zalloc(size) as an alias to calloc(1, size), use it instead.
> +
> +Fixes: bcc648a10cbcd0b9 ("btf_encoder: Encode DW_TAG_unspecified_type returning routines as void")
> +Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> +Acked-by: Andrii Nakryiko <andrii@kernel.org>
> +Acked-by: Jiri Olsa <jolsa@kernel.org>
> +Cc: bpf@vger.kernel.org
> +Cc: dwarves@vger.kernel.org
> +Link: https://lore.kernel.org/r/1666364523-9648-1-git-send-email-alan.maguire@oracle.com
> +Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> +---
> + dwarves.c | 2 +-
> + 1 file changed, 1 insertion(+), 1 deletion(-)
> +
> +diff --git a/dwarves.c b/dwarves.c
> +index fbebc1dda501..95a3bac0e36f 100644
> +--- a/dwarves.c
> ++++ b/dwarves.c
> +@@ -626,7 +626,7 @@ struct cu *cu__new(const char *name, uint8_t addr_size,
> + const unsigned char *build_id, int build_id_len,
> + const char *filename, bool use_obstack)
> + {
> +- struct cu *cu = malloc(sizeof(*cu) + build_id_len);
> ++ struct cu *cu = zalloc(sizeof(*cu) + build_id_len);
> +
> + if (cu != NULL) {
> + uint32_t void_id;
> +--
> +2.40.1
> +
> diff -Nru dwarves-1.24/debian/patches/series dwarves-1.24/debian/patches/series
> --- dwarves-1.24/debian/patches/series 2022-12-10 10:11:28.000000000 +0100
> +++ dwarves-1.24/debian/patches/series 2023-05-02 20:37:16.000000000 +0200
> @@ -1,3 +1,4 @@
> 00-store-the-CU-being-processed-to-avoid-changing-many-functions.patch
> 01-record-if-a-CU-has-a-DW_TAG_unspecified_type.patch
> 02-encode-DW_TAG_unspecified_type-returning-routines-as-void.patch
> +03-dwarves-Zero-initialize-struct-cu-in-cu__new-to-prev.patch
--
Sebastian Ramacher
Reply to: