[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <d97d3229-110f-4aee-93ad-e4499ef4133e@arm.com>
Date: Fri, 23 May 2025 10:03:58 -0500
From: Jeremy Linton <jeremy.linton@....com>
To: Nathan Chancellor <nathan@...nel.org>
Cc: linux-kbuild@...r.kernel.org, masahiroy@...nel.org,
nicolas.schier@...ux.dev, linux-kernel@...r.kernel.org,
Ard Biesheuvel <ardb@...nel.org>
Subject: Re: [PATCH] scripts: add zboot support to extract-vmlinux
Hi,
Thanks for looking at this.
On 5/22/25 6:10 PM, Nathan Chancellor wrote:
> Hi Jeremy,
>
> On Thu, May 22, 2025 at 12:29:41PM -0500, Jeremy Linton wrote:
>> Zboot compressed kernel images are used for arm kernels on various
>> distros.
>>
>> extract-vmlinux fails with those kernels because the wrapped image is
>> another PE. While this could be a bit confusing, the tools primary
>> purpose of unwrapping and decompressing the contained vmlinux image
>> makes it the obvious place for this functionality.
>>
>> Add a 'file' check in check_vmlinux() that detects a contained PE
>> image before trying readelf. Recent file implementations output
>> something like:
>>
>> "Linux kernel ARM64 boot executable Image, little-endian, 4K pages"
>>
>> Which is also a stronger statement than readelf provides so drop that
>> part of the comment. At the same time this means that kernel images
>> which don't appear to contain a compressed image will be returned
>> rather than reporting an error. Which matches the behavior for
>> existing ELF files.
>>
>> Signed-off-by: Jeremy Linton <jeremy.linton@....com>
>> Cc: Ard Biesheuvel <ardb@...nel.org>
>> ---
>> scripts/extract-vmlinux | 9 +++++----
>> 1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/scripts/extract-vmlinux b/scripts/extract-vmlinux
>> index 8995cd304e6e..edda1abe226c 100755
>> --- a/scripts/extract-vmlinux
>> +++ b/scripts/extract-vmlinux
>> @@ -12,10 +12,11 @@
>>
>> check_vmlinux()
>> {
>> - # Use readelf to check if it's a valid ELF
>> - # TODO: find a better to way to check that it's really vmlinux
>> - # and not just an elf
>> - readelf -h $1 > /dev/null 2>&1 || return 1
>> + file $1 |grep 'Linux kernel.*boot executable Image' > /dev/null
>> + if [ "$?" -ne "0" ]; then
>
> Could these two lines be simplified to:
>
> if file $1 | grep 'Linux kernel.*boot executable Image' > /dev/null; then
Yes, but it needs, a '!', which applies to the last operator in the
pipeline and for clarity should probably drop the redirection, so I
think it ends up:
if ! file $1 | grep -q 'Linux kernel.*boot executable Image'; then
But it mixes the condition checking and the execution, which is common
but maybe not the best idea.
Although, if I'm going to reroll this, i think the " Image" should be
dropped since it can also have zImage and possibly other reports in the
future and AFAIK there isn't any reason to exclude them.
>
>> + # Use readelf to check if it's a valid ELF, if 'file' fails
>> + readelf -h $1 > /dev/null 2>&1 || return 1
>> + fi
>>
>> cat $1
>> exit 0
>> --
>> 2.49.0
>>
Powered by blists - more mailing lists