From patchwork Mon Oct 23 11:45:49 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Russell King (Oracle)" X-Patchwork-Id: 10022373 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 989D3601E8 for ; Mon, 23 Oct 2017 11:46:43 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9C4D92882B for ; Mon, 23 Oct 2017 11:46:43 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 90ECF2884C; Mon, 23 Oct 2017 11:46:43 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,RCVD_IN_DNSWL_MED autolearn=unavailable version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id E0AD02882B for ; Mon, 23 Oct 2017 11:46:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=1vZrxGZ6CV/tTufgWdW+zzVr3AatM19itpnInqy04so=; b=G4TES3dzHuBvYo cN7gAbibN9JiIP7yFxk/Q6CK3vB0+RK4NIfsQHW5zwfQT/bORM28jO7oe915wQGAkXAOyhEw278iN 3M2FTpPzHx/JxklCK++gaVmhPOjPP6H6nSqycWqmGqToXI0kNsyRTp+7suD019qHXRrBzij5URB7v Q3SjMvHdNaquJ/+9ci3GBG+mIPDKk1/ogaddyrUSCQ6lMQ1sd9y3sYoSm6GT88tVUExlA4cja412w gLK9NkaxwzmFJo9p7wGwJ1a3BJ+caFM54l7BAExjh7aLy0gRBWaGlBT7/8MCi1taKXJ4utNsUnHsh A0Ydr3I+X0DgGh2b/kFA==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1e6bBk-0001Y8-9v; Mon, 23 Oct 2017 11:46:36 +0000 Received: from pandora.armlinux.org.uk ([2001:4d48:ad52:3201:214:fdff:fe10:1be6]) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1e6bBc-0001A0-JU for linux-arm-kernel@lists.infradead.org; Mon, 23 Oct 2017 11:46:32 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=armlinux.org.uk; s=pandora-2014; h=Sender:In-Reply-To:Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date; bh=oSJ3npZwCYqx1ffXEcM5ztimm6LWIgbCqUi6PDbL2YQ=; b=aUw9G3aZE4bbLTubOvpcQd8niVkbDkrOzKz1tL9iM4fb8AkJXRZVma691BEKlJ2KR87xFVu9oD44rrLjJfuxgxys7jOfBuquym8GT9k31g9n3UsMQq5e+EKkzSrWruywbKqNpedOBil6bHGztEmGGqcSTFRJLn44hD6iSFHRhNw=; Received: from n2100.armlinux.org.uk ([2002:4e20:1eda:1:214:fdff:fe10:4f86]:40426) by pandora.armlinux.org.uk with esmtpsa (TLSv1:DHE-RSA-AES256-SHA:256) (Exim 4.82_1-5b7a7c0-XX) (envelope-from ) id 1e6bB4-00075Q-05; Mon, 23 Oct 2017 12:45:54 +0100 Received: from linux by n2100.armlinux.org.uk with local (Exim 4.76) (envelope-from ) id 1e6bB0-0002OS-1A; Mon, 23 Oct 2017 12:45:50 +0100 Date: Mon, 23 Oct 2017 12:45:49 +0100 From: Russell King - ARM Linux To: jeffy Subject: Re: [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled Message-ID: <20171023114549.GU20805@n2100.armlinux.org.uk> References: <20171018050108.10352-1-jeffy.chen@rock-chips.com> <20171022124757.GL20805@n2100.armlinux.org.uk> <59ED6179.5020301@rock-chips.com> <20171023085006.GM20805@n2100.armlinux.org.uk> <59EDC34C.40109@rock-chips.com> <20171023105046.GT20805@n2100.armlinux.org.uk> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20171023105046.GT20805@n2100.armlinux.org.uk> User-Agent: Mutt/1.5.23 (2014-03-12) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20171023_044629_157312_73BA475E X-CRM114-Status: GOOD ( 35.28 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "linux-kernel@vger.kernel.org" , Ard Biesheuvel , Ingo Molnar , "linux-arm-kernel@lists.infradead.org" , chris.zhong@rock-chips.com Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP On Mon, Oct 23, 2017 at 11:50:46AM +0100, Russell King - ARM Linux wrote: > On Mon, Oct 23, 2017 at 06:24:12PM +0800, jeffy wrote: > > Hi Russell, > > > > Thanks for your reply. > > > > On 10/23/2017 04:50 PM, Russell King - ARM Linux wrote: > > >>> > > >>>hmm, right, didn't notice the data is already aligned... > > >>>so it's indeed caused by the ksym: > > >>> > > >>> [ 9] .data PROGBITS 006ce000 6d6000 000200 00 WA 0 > > >>>0 4096 > > >>> [10] ___ksymtab+sort PROGBITS 006ce200 6d6200 000008 00 WA 0 > > >>>0 4 > > >>> [11] .bss NOBITS 006ce208 6d6208 00001c 00 WA 0 > > >>>0 4 > > >It's earlier - look for __ksymtab_strings. > > > > the problem i meet is the appended dtb code found dtb invalid. i thought > > that is because of unaligned zImage size, but i was wrong... > > Hmm, you really ought not to be using the appended dtb code for modern > systems - the appended dtb system is there for old boot loaders that > are incapable of dealing with a dtb. As is said in the option's help > text: > > This is meant as a backward compatibility convenience for those > systems with a bootloader that can't be upgraded to accommodate > the documented boot protocol using a device tree. > > Beware that there is very little in terms of protection against > this option being confused by leftover garbage in memory that might > look like a DTB header after a reboot if no actual DTB is appended > to zImage. Do not leave this option active in a production kernel > if you don't intend to always append a DTB. Proper passing of the > location into r2 of a bootloader provided DTB is always preferable > to this option. > > If you rely on it, and you have something that looks like a dtb after > the image, then things will go wrong, so it's better _not_ to use it > and to keep it disabled. > > That aside, thanks for doing a more in-depth analysis of what is going > on, which helps to understand /why/ Ard's fix works (whereas before > it was rather nebulous.) > > I wonder whether we ought to tell the linker to discard any unknown > sections by adding at the bottom: > > /DISCARD/ { *(*) } > > but I do think we need to document this, specifically that _edata must > point to the first byte after the binary file, and that the only > sections after it are allowed to be the .bss and stack sections. Short of adding the discard (which I think is itself risky, we've had problems in the main kernel's vmlinux.lds in this area), I think we ought to verify the size of the zImage file, so that the build fails when we generate a zImage which is wrong, rather than producing a zImage that is incorrect. Maybe something like this? 8<===== From: Russell King Subject: [PATCH] ARM: verify size of zImage The linker can sometimes add additional sections to the zImage ELF file which results in the zImage binary being larger than expected. This causes appended DT blobs to fail. Verify that the zImage binary is the expected size, and fail the build if this is not the case. Signed-off-by: Russell King --- arch/arm/boot/Makefile | 6 +++++- arch/arm/boot/verify_zimage.sh | 21 +++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) create mode 100755 arch/arm/boot/verify_zimage.sh diff --git a/arch/arm/boot/Makefile b/arch/arm/boot/Makefile index a3af4dc08c3e..1290875556ae 100644 --- a/arch/arm/boot/Makefile +++ b/arch/arm/boot/Makefile @@ -53,6 +53,10 @@ $(obj)/Image $(obj)/zImage: FORCE else +quiet_cmd_mkzimage = ZIMAGE $@ +cmd_mkzimage = $(cmd_objcopy) && $(CONFIG_SHELL) -c \ + '$(srctree)/$(src)/verify_zimage.sh $< $@ "$(NM)" || { rm -f $@; false; }' + $(obj)/xipImage: FORCE @echo 'Kernel not configured for XIP (CONFIG_XIP_KERNEL!=y)' @false @@ -64,7 +68,7 @@ $(obj)/compressed/vmlinux: $(obj)/Image FORCE $(Q)$(MAKE) $(build)=$(obj)/compressed $@ $(obj)/zImage: $(obj)/compressed/vmlinux FORCE - $(call if_changed,objcopy) + $(call if_changed,mkzimage) endif diff --git a/arch/arm/boot/verify_zimage.sh b/arch/arm/boot/verify_zimage.sh new file mode 100755 index 000000000000..922a93e61aa7 --- /dev/null +++ b/arch/arm/boot/verify_zimage.sh @@ -0,0 +1,21 @@ +#!/bin/sh +set -e + +vmlinuz="$1" +zimage="$2" +nm="$3" + +magic_size=$("$nm" "$vmlinuz" | perl -e 'while (<>) { + $magic_start = hex($1) if /^([[:xdigit:]]+) . _magic_start$/; + $magic_end = hex($1) if /^([[:xdigit:]]+) . _magic_end$/; +}; printf "%d\n", $magic_end - $magic_start;') + +zimage_size=$(stat -c '%s' "$zimage") + +# Verify that the resulting binary matches the size contained within +# the binary (iow, the linker has not added any additional sections.) +if [ $magic_size -ne $zimage_size ]; then + echo "zImage size ($zimage_size) disagrees with linked size ($magic_size)" >&2 + exit 1 +fi +exit 0