From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gnu.wildebeest.org (wildebeest.demon.nl [212.238.236.112]) by sourceware.org (Postfix) with ESMTPS id 98C823951815 for ; Wed, 7 Jul 2021 12:02:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 98C823951815 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=klomp.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=klomp.org Received: from tarox.wildebeest.org (83-87-18-245.cable.dynamic.v4.ziggo.nl [83.87.18.245]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by gnu.wildebeest.org (Postfix) with ESMTPSA id 5AA58302FBA6; Wed, 7 Jul 2021 14:02:31 +0200 (CEST) Received: by tarox.wildebeest.org (Postfix, from userid 1000) id C0D774000B17; Wed, 7 Jul 2021 14:02:30 +0200 (CEST) Message-ID: <9d78443192899df450e3f1128b7692c55e3e6bf6.camel@klomp.org> Subject: Re: [PATCH] libelf: Fix unaligned d_off offsets for input sections with large alignments From: Mark Wielaard To: Andrei Homescu , elfutils-devel@sourceware.org Date: Wed, 07 Jul 2021 14:02:30 +0200 In-Reply-To: <20210629012653.2375942-1-ah@immunant.com> References: <20210629012653.2375942-1-ah@immunant.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Mailer: Evolution 3.28.5 (3.28.5-10.el7) Mime-Version: 1.0 X-Spam-Status: No, score=-4.5 required=5.0 tests=BAYES_00, KAM_DMARC_STATUS, KAM_SHORT, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: elfutils-devel@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Elfutils-devel mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 07 Jul 2021 12:02:35 -0000 Hi Andrei, On Mon, 2021-06-28 at 18:26 -0700, Andrei Homescu wrote: > The mkl_memory_patched.o object inside the libmkl_core.a library from > the Intel Math Kernel Library version 2018.2.199 has this section > with an alignment of 4096 and offset of 0xb68: > [ 2] .data PROGBITS 0000000000000000 000b68 011000 00 WA 0 0 4096 >=20 > Reading this file with libelf and trying to write it back to disk trigger= s > the following sequence of events: > 1) code in elf_getdata.c clamps d_align for this section's data buffer > to the section's offset > 2) code in elf32_updatenull.c checks if the alignment is a power of two > and incorrectly returns an error >=20 > This commit fixes this corner case by increasing the alignment to the > next power of two after the clamping, so the check passes. >=20 > A test that reproduces this bug using strip is also included. Thanks for this perfect patch, including and excellent explanation of the issue and a testcase. I have only a minor suggestion: > --- /dev/null > +++ b/tests/run-strip-largealign.sh > @@ -0,0 +1,34 @@ > +#! /bin/sh > +# Copyright (C) 2011 TODO. Should we make this Copyright (C) 2021 Andrei Homescu or Copyright (C) 2021 Immunant, Inc. > +# This file is part of elfutils. > +# > +# This file is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published > by > +# the Free Software Foundation; either version 3 of the License, or > +# (at your option) any later version. > +# > +# elfutils is distributed in the hope that it will be useful, but > +# WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see < > http://www.gnu.org/licenses/>. > +# > + > +. $srcdir/test-subr.sh > + > +# =3D testfile-largealign.S =3D > +# section .data > +# align 4096 > +# dd 0x12345678 > +# > +# nasm -f elf64 -o testfile-largealign.o testfile-largealign.S > + > +infile=3Dtestfile-largealign.o > +outfile=3D$infile.stripped > + > +testfiles $infile > +tempfiles $outfile > + > +testrun ${abs_top_builddir}/src/strip -o $outfile $infile The testcase already fails before the patch and succeeds after, but it would be nice to double check the output with elflint just in case. Shall we add: testrun ${abs_top_builddir}/src/elflint --gnu $outfile Thanks, Mark