From f4dc76404ce8116b806550d7515ec98be953df9e Mon Sep 17 00:00:00 2001 From: Mark Wielaard Date: Tue, 9 Feb 2016 14:18:49 +0100 Subject: [PATCH] elflint: Fix sh_entsize check when comparing SHT_HASH and SHT_GNU_HASH. MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit GCC6 -Wduplicated-cond found the following issue: elflint.c: In function ‘compare_hash_gnu_hash’: elflint.c:2483:34: error: duplicated ‘if’ condition [-Werror=duplicated-cond] else if (hash_shdr->sh_entsize == sizeof (Elf64_Word)) ~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~ elflint.c:2448:29: note: previously used here if (hash_shdr->sh_entsize == sizeof (Elf32_Word)) ~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~ Which is correct, a Word in both Elf32 and Elf64 files is 4 bytes. We meant to check for sizeof (Elf64_Xword) which is 8 bytes. Also fix the section index and name in the error message. The reason we probably didn't see this issue before is that SHT_HASH sections really always should have sh_entsize of 4 even on 64bit arches. There are however a couple of arches with mistakes in their sysv ABI. See libelf/common.h. This also would only be triggered if on such an architectures when the ELF file would have both a SHT_HASH and SHT_GNU_HASH section and elflint would try to compare those sections. Add an example testfile-s390x-hash-both to run-elflint-test.sh. Signed-off-by: Mark Wielaard --- src/ChangeLog | 6 ++++++ src/elflint.c | 4 ++-- tests/ChangeLog | 6 ++++++ tests/Makefile.am | 1 + tests/run-elflint-test.sh | 9 ++++++++- tests/testfile-s390x-hash-both.bz2 | Bin 0 -> 2435 bytes 6 files changed, 23 insertions(+), 3 deletions(-) create mode 100755 tests/testfile-s390x-hash-both.bz2 diff --git a/src/ChangeLog b/src/ChangeLog index 707c27177..e4b17d623 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,9 @@ +2016-02-09 Mark Wielaard + + * elflint.c (compare_hash_gnu_hash): Check hash sh_entsize against + sizeof (Elf64_Xword). Correct invalid sh_entsize error message + section idx and name. + 2016-01-13 Mark Wielaard * elflint.c (check_elf_header): Recognize ELFOSABI_FREEBSD. diff --git a/src/elflint.c b/src/elflint.c index eae776143..15b12f6f5 100644 --- a/src/elflint.c +++ b/src/elflint.c @@ -2482,7 +2482,7 @@ hash section [%2zu] '%s' uses too much data\n"), } } } - else if (hash_shdr->sh_entsize == sizeof (Elf64_Word)) + else if (hash_shdr->sh_entsize == sizeof (Elf64_Xword)) { const Elf64_Xword *hasharr = (Elf64_Xword *) hash_data->d_buf; if (hash_data->d_size < 2 * sizeof (Elf32_Word)) @@ -2523,7 +2523,7 @@ hash section [%2zu] '%s' uses too much data\n"), { ERROR (gettext ("\ hash section [%2zu] '%s' invalid sh_entsize\n"), - gnu_hash_idx, gnu_hash_name); + hash_idx, hash_name); return; } diff --git a/tests/ChangeLog b/tests/ChangeLog index a043ade6d..bcc296f14 100644 --- a/tests/ChangeLog +++ b/tests/ChangeLog @@ -1,3 +1,9 @@ +2016-02-09 Mark Wielaard + + * testfile-s390x-hash-both.bz2: New testfile. + * Makefile.am (EXTRA_DIST): Add testfile-s390x-hash-both.bz2. + * run-elflint-test.sh: Add elflint testfile-s390x-hash-both test. + 2016-02-04 Mark Wielaard * run-strip-nobitsalign.sh: New test. diff --git a/tests/Makefile.am b/tests/Makefile.am index f3e7c01ca..fedcb39d2 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -236,6 +236,7 @@ EXTRA_DIST = run-arextract.sh run-arsymtest.sh \ testfile56.bz2 testfile57.bz2 testfile58.bz2 \ run-typeiter.sh testfile59.bz2 \ run-readelf-d.sh testlib_dynseg.so.bz2 \ + testfile-s390x-hash-both.bz2 \ run-readelf-gdb_index.sh testfilegdbindex5.bz2 \ testfilegdbindex7.bz2 \ run-readelf-s.sh testfilebazdbg.bz2 testfilebazdyn.bz2 \ diff --git a/tests/run-elflint-test.sh b/tests/run-elflint-test.sh index 68615b961..f3bd9012c 100755 --- a/tests/run-elflint-test.sh +++ b/tests/run-elflint-test.sh @@ -1,5 +1,5 @@ #! /bin/sh -# Copyright (C) 2005, 2007, 2008 Red Hat, Inc. +# Copyright (C) 2005, 2007, 2008, 2015 Red Hat, Inc. # This file is part of elfutils. # Written by Ulrich Drepper , 2005. # @@ -40,4 +40,11 @@ testrun ${abs_top_builddir}/src/elflint -q testfile46 testfiles testlib_dynseg.so testrun ${abs_top_builddir}/src/elflint -q --gnu-ld testlib_dynseg.so +# s390x has SHT_HASH with sh_entsize 8 (really should be 4, but see common.h) +# This was wrongly checked when comparing .gnu.hash and .hash. +# Simple "int main (int argc, char **argv) { return 0; }" +# gcc -Xlinker --hash-style=both -o testfile-s390x-hash-both s390x-hash-both.c +testfiles testfile-s390x-hash-both +testrun ${abs_top_builddir}/src/elflint -q --gnu-ld testfile-s390x-hash-both + exit 0 diff --git a/tests/testfile-s390x-hash-both.bz2 b/tests/testfile-s390x-hash-both.bz2 new file mode 100755 index 0000000000000000000000000000000000000000..86e0bcc792da1aa7f74c78c6a3de13f3b75b3723 GIT binary patch literal 2435 zc-oba={FmQ0>u*{4H{ytgfT*j+MXq7=g~=sG>A@$smPhST|Z;s>gim_(m>$$WYpo>j^n!J2M7rgv75UM9Ob^t1=`jaY@u<=sHq1tP-OsG zNjZU;k&i$iK=@c96N&~irZH#$6pC;|fD09dYoFvft03^V`%0I~tQG2jGKQ-vuu0%Ub&8Datk#77G@MSE|esRN=mDo;z!VgMAd&yX*{pg|b# zOg7-G7J&dKuz)2d|3|kZ5_0f4i$000_(2CYtT z$1lF4iDbP;7yYbp@)09_Wd)pciP|4gPX{huYw1Z@O;R^=w1tfDH>Um6l3p|R@tqW> zIdWH2e%IV^ieSVr(Y9e1=wh8lBAs{i8SCfdzK}nQ@X~}uR|FdLDm>L^_YjxHQ~@7qTj)v}^_fQzl*y zTWmYGM@P?9UoXuzCtO#L?0#_)sd4B7WW;Q~>k z#2rh{dN;Cq1c2gB!VtWf6uAG20=L^4oQl35Da-^v2HTP@PQbCwrNQ@*+%JqzZFOOayCQ44D0 z1`$LPT~`A>Uw#9ItgNZ4s>_pG1}@fJ8+LC?)I#RN6f1+1$pnMM1i)$*Vd!1AQVEj> zf_d5RoX3Fgc=4;xE9z}Iy(Cz~DNjq6&xiwN^^t81D|Ai;{p8@HnuK>OQ7u|Q2ix*+ zLP?STd1@>IvY>wRwqzSipLSZqL33EM4cf{>+Im55 zm7L_EpCgC%RbZnUm46T7XM<;rn~QsHbiA=VraP6**-z0k1%;(z10pipd;UJUC+tvw zyaSS_3w`kDzv?{_f6c<#C%3Q@Hj&;vJv_R-9~l|CgPOacP3?|wc9ME4=}$O;3%jlg zC2=+1l=YJgZT_53tUb~R*y8&U|5vc+rLr+}OqzJO_fn>|Et|wy12_*Bf5Ox@_{!F}r-3;Souv@^1IZ$pu~v+x1R??7 zGG1b3*y^sohw>7$7EUwuZdvfido7~t9q8xJw2ka54Y+`ZS-qm1Gg?Vj3tYsHR>Ua@ zin*zpg=*-uU(uh;xu9@Ask@q~72W1?1>Q-y@vpaBzXa#yz z9c?HPK3l3ss@g5gLv)Mcb(-($FuN$_4seL*`;+yX@e23!7?>q?(K*K@<7@i6f~Zt2 z+G^^Ig1~s(9Mo!fZ;j&p+Q~eyH*IlY>As=>!naRRub>suDh3x}e`vNXYGV0N&f&?v z%O*WxValkMr*cifJkjqV$NE>-DkPv^7G8M@Y?JD?#O{pOne)Aq!@fm-;Lp#5Yz(Fn zyzoiZ>$h|{_HMQ}RSsRYbhm#im8!mf_po(+eFuCCZ3l7sXgYT5hA*pc4xzWl%Uj zF)5@vUsS41t3C=+g~M6F;fvfNlD&QWih_cI%yFrphNlZgC%$la%7Iipok;lEfRZSg z{N)ohEoIHND!B+E@XSbtYA3wWU-7wN{o7(OPosPA`Y#RyB3aj9jZV8#+cBw-KG8-? zJ{;mCq|r9DCI5mJ$WYcudjyf>iX5o4l^?ERTyxw}pU!vr?rqki-b7-m&(T!6DBrk7 z{3Dqch5SOpN)5O{z9ARNAF9-(K*X_JzPA`Mb#;fU(jDK;oNI({di3BdCj#)j0!XGc zdL}zA_a{^}rdJJ*D|`Ta^c&vpl3Gqr?icrtH_9H%KtPt>S|PK&vgsz)3{GNkhdmo_ zKl$}8Z{S%jbAFe#0MoD) z!e!!;(e4rwX4wLJuy8Pa6YH8$C)IG_Xfnx7v(xW{X_Wsl#<(C1Q!i)z25=n|Eod%b zc2htV$9~SPxkVwyq!88}u*3v=4xvFw^*oc8YLIL;F_mZdY`+#13I~v3FOMyY)nyv^ zs>M5QE89rEP$hsbI51uo_byyUd$5C)yPU*KpkPZ;Mom{&*X&v_3M~dpf-&UwVD}y3 zrd#>b^B>yOS1mZq(JFZpE5e57*$E_y>Er`FgGL?2Tfw0>s}qUp#?%a{!oH1k(L#$Q z4V1(~!>QAx_)7l?=R6rKg=JqNa`%o@GfgEut3$k~l@#P=li0W(Sr+$}nX`BOquC5M zfmdpD>fuCoXizK|_b}p-$lvm$z0&8!?HqT_P)42d!}FJs&w$cmKVWlyt2j0@Eeq}M zX{Tlk7`R8S#kJ`o-M`TYX(67T^jiXm tll}9wa7S75;5^eijV)>Y{Qb0LnziJ9m?wFp?>~Q_Y~lq)Rr2cD{{`>`StkGh literal 0 Hc-jL100001 -- 2.47.2