From patchwork Fri Mar 20 22:19:07 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dodji Seketeli X-Patchwork-Id: 39051 From: dodji@seketeli.org (Dodji Seketeli) Date: Fri, 20 Mar 2020 23:19:07 +0100 Subject: [PATCH] dwarf-reader: Fix bloom filter access in GNU_HASH section Message-ID: <86d096sm90.fsf@seketeli.org> Hello, The bloom filter of the GNU_HASH section of binaries is made of words (bitmasks) that are either 32-bits for ELFCLASS32 binaries or 64-bits for ELFCLASS64 binaries. By using the GElf_Word type to hold the content of each bitmask we assume the bitmask to be of a size of at most 'sizeof(Elf64_Word)'. Unfortunately, the name Elf64_Word is a bit misleading because it's a 32-bits wide type (uint32_t). That type is thus too short to hold an entire bitmask for 64-bits binaries. I won't give too many details here about how the bloom filter works as it's described in details in the documentation for the GNU_HASH section at http://www.linker-aliens.org/blogs/ali/entry/gnu_hash_elf_sections/. Suffice it to say that in practise, we were comparing the least significant bytes of a 64-bits bloom bitmask to a 32-bits value. Depending on if we read those least significant bytes on a big or little endian, we obviously don't get the same result. Hence the recent buid breakage at https://builder.wildebeest.org/buildbot/#builders/14/builds/352 where the runtestlookupsyms test fails. This patch thus changes the code of lookup_symbol_from_gnu_hash_tab to use a 64-bits type to hold the value of the bloom bitmask. That way, we never have any information loss. The function bloom_word_at is also changed to read the bloom bitmask as a 64-bits value when looking at an ELFCLASS64 binary and to always return a 64-bits value. It also adds a test to access the bloom filter of an ELFCLASS32 binary. * src/abg-dwarf-reader.cc (bloom_word_at): Properly read an element from the bloom bitmasks array of 32-bits values as a 64-bits value in a portable way. Always return a 64 bits value. (lookup_symbol_from_gnu_hash_tab): Use a 64-bits value to store the bloom bitmask. * tests/data/test-lookup-syms/test1-32bits.so: New test input, compiled as a 32bits binary to test for ELFCLASS32 binaries. * tests/data/test-lookup-syms/test1.c.compiling.txt: Documentation about how to compile the test1[-21bits].so files. * tests/data/Makefile.am: Add the new test material above to source distribution. * tests/test-lookup-syms.cc (in_out_specs): Add the test1-32bits.so test case to this test harness. Signed-off-by: Dodji Seketeli --- src/abg-dwarf-reader.cc | 24 ++++++++++++++++------ tests/data/Makefile.am | 2 ++ tests/data/test-lookup-syms/test1-32bits.so | Bin 0 -> 7800 bytes tests/data/test-lookup-syms/test1.c.compiling.txt | 9 ++++++++ tests/test-lookup-syms.cc | 7 +++++++ 5 files changed, 36 insertions(+), 6 deletions(-) create mode 100755 tests/data/test-lookup-syms/test1-32bits.so create mode 100644 tests/data/test-lookup-syms/test1.c.compiling.txt diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc index 5bc8a15..10ca123 100644 --- a/src/abg-dwarf-reader.cc +++ b/src/abg-dwarf-reader.cc @@ -1850,20 +1850,27 @@ get_elf_class_size_in_bytes(Elf* elf_handle) } /// Get a given word of a bloom filter, referred to by the index of -/// the word. The word size depends on the current elf class and this -/// function abstracts that nicely. +/// the word. +/// +/// The bloom word size depends on the current elf class (32 bits for +/// an ELFCLASS32 or 64 bits for an ELFCLASS64 one) and this function +/// abstracts that nicely. /// /// @param elf_handle the elf handle to use. /// /// @param bloom_filter the bloom filter to consider. /// /// @param index the index of the bloom filter to return. -static GElf_Word +/// +/// @return a 64 bits work containing the bloom word found at index @p +/// index. Note that if we are looking at an ELFCLASS32 binary, the 4 +/// most significant bytes of the result are going to be zero. +static Elf64_Xword bloom_word_at(Elf* elf_handle, Elf32_Word* bloom_filter, size_t index) { - GElf_Word result = 0; + Elf64_Xword result = 0; GElf_Ehdr h; ABG_ASSERT(gelf_getehdr(elf_handle, &h)); int c; @@ -1876,7 +1883,7 @@ bloom_word_at(Elf* elf_handle, break ; case ELFCLASS64: { - GElf_Word* f= reinterpret_cast(bloom_filter); + Elf64_Xword* f= reinterpret_cast(bloom_filter); result = f[index]; } break; @@ -2025,7 +2032,12 @@ lookup_symbol_from_gnu_hash_tab(const environment* env, // filter, in bits. int c = get_elf_class_size_in_bytes(elf_handle) * 8; int n = (h1 / c) % ht.bf_nwords; - GElf_Word bitmask = (1ul << (h1 % c)) | (1ul << (h2 % c)); + // The bitmask of the bloom filter has a size of either 32-bits on + // ELFCLASS32 binaries or 64-bits on ELFCLASS64 binaries. So we + // need a 64-bits type to hold the bitmap, hence the Elf64_Xword + // type used here. When dealing with 32bits binaries, the upper + // bits of the bitmask will be zero anyway. + Elf64_Xword bitmask = (1ul << (h1 % c)) | (1ul << (h2 % c)); // Test if the symbol is *NOT* present in this ELF file. if ((bloom_word_at(elf_handle, ht.bloom_filter, n) & bitmask) != bitmask) diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am index e9fdad1..cf7792f 100644 --- a/tests/data/Makefile.am +++ b/tests/data/Makefile.am @@ -1324,6 +1324,8 @@ test-lookup-syms/test02-report.txt \ test-lookup-syms/test1.c \ test-lookup-syms/test1.version-script \ test-lookup-syms/test1.so \ +test-lookup-syms/test1-32bits.so \ +test-lookup-syms/test1.c.compiling.txt \ test-lookup-syms/test1-1-report.txt \ test-lookup-syms/test1-2-report.txt \ test-lookup-syms/test1-3-report.txt \ diff --git a/tests/data/test-lookup-syms/test1-32bits.so b/tests/data/test-lookup-syms/test1-32bits.so new file mode 100755 index 0000000000000000000000000000000000000000..119cee41f49cd5344fb71b4e0b8d9c95b3afc43f GIT binary patch literal 7800 zcmeHMYitzP6~41$!zMNx15OAblqq!*sOs?phhiX!jbE6WM{ScfO3|?E9j^y=cj(S6 zF(^`?3jxb=OBGeA)Tj@ks6V2rk;0!g{bA%ZrBYOZTB+%eKGcX73aVTwZD<2=zwh3; z_ToYt^|z0G=G=47cVB1jy>sr}7rOeo4Z{$cOkoOY7fm5rAor|PmKI@&#bTZKB;*`g z+1{xXf{cG5YJ{{`+$TiE8pUxGs0J^J>@mn%*lS@YzYCpAdqu>xz-z8U9$DM=Tn^Lg z(3s}=_Rqr(_j)9xuH=Gc$8? zGOWDvC^AcL#h;ymR5bGjM?Q0w?U82AP6IrAX67sl4WAi*xNd6r%-rAJvC9Kj{tVOc zsTHWZbiH<>grz4YUaH1({l(IaMb9lNy;b|{zNzujb=B_lSZ&?R#cNk5!bgQH|9x5h z*Q=uPEUC+%zY!ZdfBxzku7Z_ax^{Wu90#N7`+}_Rl&bI8 zo2*Y1>&8nQ3dWt7DAgb}R#A6lHRP2|c)VRA%1}LJ$0mOA7>mjNEe7ED)Mr(FzeFQ3sj(5dNlg7R&os8|Hr{(*2<0IDj-b2v~FjkpcEZFe+ z#A%qeLjDq!?1%gvJYxP)>6Sf3*!tP1K@IC?3=X2J{M)vLB=c8%)?Q3 z?CWTtqSDKUqH-tAi)q#;UE>u3Wfp>=n^KQW#f|@K0jXW{B zX=DAy7OOrZ92}#YMZ<`na~slr`heGv@xlh2tCJ}&lk~C;!``SHH1K#GG^CSZQYP!O zzJIWIs6IH73ugPus+v@f=RJA^6pw^z!aoh&IK5OTS5@K>z=fqKWR+Oe(xoXWSohw+~7b!g< z^*+@t;CUj(`;XRw8J+Zbo+p`drD8vr)wyETUO{Qfm5cUf?dl}R^S`p+uM(CLX0%YM zXLZceGBO1wV;Pxp=b6he1$Uk~GUbj`u5(A4BU7xFdz#LvzOVm0LiBzNyq`Crm-knfr6m4mki4^GlkDGbA@wdn{XOW~-`uGF0-bw0DTW<>8j^cI z$-w)(1zBTJBdriIbWq=yu|9dHX-T8B%e%_i^0i_$@VuW?ln{;3b^8X|-^u}7%1XT- z`iWWp-y^Ci6(PO>;EhWy;2 ztQ)#%dt-BJ%bh{IJoKDop^!Y{xcRVfL<|>_IoC-SbGaiZQl>dThPOfGZKCX8cYpiN zE~jf(C$BPPcXb+jsGV^`=g^Q7jui6;?bK+rn>sExN+_t8oeO+tB$-e1!qd&CC8!Dc zsUT3dq{8WZW>@>p-VS79b9cJ7JF403o&Ctdm8tGPPv7os?S0Pf?(TuEL1(aiTVEHt z#qR(2d)C`-Q`_3gFF~j_b$9u3T0Mq&AWaMueR{l1E7;!-BFLb_;>$=WkP-s8nQg04=y#78kbk zi+v~?+8I9-cIb}cDPM~1jzkk+B#jbUQ$^HFP)MCVFa#Lis;s@>%Z?>;-Vi$DhipUG zc%Qg9)7RGCt#f_qqNSugMEG#rG5^D&A0E5CaDB+9)5 z>#eS=Y_xUCefJ-T^N8d3m)wYSv{lIcj@V6Leg%o+cbWWMq-jG^O2(dTV3sA0F)H~r zq%{#s+74#CNgTi9WPa6jyUZu^cL~O+O!NCr{u0t`msmZHG}4S?iQ{*lY(;k6Kd2%A z^J`BW;{kFT(h!nZA7kX_!7RrZ`vi>SPa&;IxBDk#XdL4QGX0>5X=1+u*0@Pv$mKY; zNg4-h9KSc@c4kDR%gAUHdF&s*Gm4z0ws8;8 zX_|fkTv~%5wQcP@T=P=3_BB-F>u}clt8NULI*Deak2UTAgtD#LScdyIPhVNs literal 0 HcmV?d00001 diff --git a/tests/data/test-lookup-syms/test1.c.compiling.txt b/tests/data/test-lookup-syms/test1.c.compiling.txt new file mode 100644 index 0000000..6753d63 --- /dev/null +++ b/tests/data/test-lookup-syms/test1.c.compiling.txt @@ -0,0 +1,9 @@ +To re-compile test1.so (on a 64-bits machine), please do: + +gcc -g -shared -Wl,--version-script=test1.version-script -o test1.so test1.c + + +To re-compile test1-32.so, please do: + +gcc -g -m32 -shared -Wl,--version-script=test1.version-script -o test1.so test1.c + diff --git a/tests/test-lookup-syms.cc b/tests/test-lookup-syms.cc index 2f4d8f7..077f33b 100644 --- a/tests/test-lookup-syms.cc +++ b/tests/test-lookup-syms.cc @@ -74,6 +74,13 @@ InOutSpec in_out_specs[] = "output/test-lookup-syms/test1-1-report.txt" }, { + "data/test-lookup-syms/test1-32bits.so", + "foo", + "", + "data/test-lookup-syms/test1-1-report.txt", + "output/test-lookup-syms/test1-1-report.txt" + }, + { "data/test-lookup-syms/test1.so", "_foo1", "--no-absolute-path",