* [PATCH] libdwfl: Correctly handle corefile non-contiguous segments
@ 2023-11-12 20:16 Aaron Merey
2023-11-13 14:44 ` Aaron Merey
2023-11-14 14:03 ` Mark Wielaard
0 siblings, 2 replies; 4+ messages in thread
From: Aaron Merey @ 2023-11-12 20:16 UTC (permalink / raw)
To: elfutils-devel; +Cc: Aaron Merey
It is possible for segments of different shared libaries to be interleaved
in memory such that the segments of one library are located in between
non-contiguous segments of another library.
For example, this can be seen with firefox on RHEL 7.9 where multiple
shared libraries could be mapped in between ld-2.17.so segments:
[...]
7f0972082000-7f09720a4000 00000000 139264 /usr/lib64/ld-2.17.so
7f09720a4000-7f09720a5000 00000000 4096 /memfd:mozilla-ipc (deleted)
7f09720a5000-7f09720a7000 00000000 8192 /memfd:mozilla-ipc (deleted)
7f09720a7000-7f09720a9000 00000000 8192 /memfd:mozilla-ipc (deleted)
7f0972134000-7f0972136000 00000000 8192 /usr/lib64/firefox/libmozwayland.so
7f0972136000-7f0972137000 00002000 4096 /usr/lib64/firefox/libmozwayland.so
7f0972137000-7f0972138000 00003000 4096 /usr/lib64/firefox/libmozwayland.so
7f0972138000-7f0972139000 00003000 4096 /usr/lib64/firefox/libmozwayland.so
7f097213a000-7f0972147000 00000000 53248 /usr/lib64/firefox/libmozsqlite3.so
7f0972147000-7f097221e000 0000d000 880640 /usr/lib64/firefox/libmozsqlite3.so
7f097221e000-7f0972248000 000e4000 172032 /usr/lib64/firefox/libmozsqlite3.so
7f0972248000-7f0972249000 0010e000 4096 /usr/lib64/firefox/libmozsqlite3.so
7f0972249000-7f097224c000 0010e000 12288 /usr/lib64/firefox/libmozsqlite3.so
7f097224c000-7f0972250000 00111000 16384 /usr/lib64/firefox/libmozsqlite3.so
7f0972250000-7f0972253000 00000000 12288 /usr/lib64/firefox/liblgpllibs.so
[...]
7f09722a3000-7f09722a4000 00021000 4096 /usr/lib64/ld-2.17.so
7f09722a4000-7f09722a5000 00022000 4096 /usr/lib64/ld-2.17.so
dwfl_segment_report_module did not account for the possibility of
interleaving non-contiguous segments, resulting in premature closure
of modules as well as failing to report modules.
Fix this by removing segment skipping in dwfl_segment_report_module.
When dwfl_segment_report_module reported a module, it would return
the index of the segment immediately following the end address of the
current module. Since there's a chance that other modules might fall
within this address range, dwfl_segment_report_module instead returns
the index of the next segment.
This patch also fixes premature module closure that can occur in
dwfl_segment_report_module when interleaving non-contiguous segments
are found. Previously modules with start and end addresses that overlap
with the current segment would have their build-id compared the build-id
associated with the current segment. If there was a mismatch, that module
would be closed. Avoid closing modules in this case when mismatching
build-ids correspond to distinct modules.
A couple caveats should be mentioned. First, start and end addresses
of reported modules cannot be assumed to contain segments from only
that module. This has always been the case however.
Second, the testcases in this patch use a firefox corefile that is
fairly large. The .bz2 corefile is about 47M. A clean elfutils repo
is currently about 42M, so this corefile more than doubles the size of
the elfutils repo. I looked for a much smaller process with
interleaving non-contiguous shared library sections but was not able
to find one. I've included the corefile and tests in this patch but
they can be removed if we'd prefer to not approx. double the size of
the repo.
https://sourceware.org/bugzilla/show_bug.cgi?id=30975
Signed-off-by: Aaron Merey <amerey@redhat.com>
---
libdwfl/dwfl_segment_report_module.c | 37 +++++--
tests/Makefile.am | 5 +-
tests/run-unstrip-noncontig.sh | 155 +++++++++++++++++++++++++++
tests/testcore-noncontig.bz2 | Bin 0 -> 49146091 bytes
4 files changed, 184 insertions(+), 13 deletions(-)
create mode 100755 tests/run-unstrip-noncontig.sh
create mode 100644 tests/testcore-noncontig.bz2
diff --git a/libdwfl/dwfl_segment_report_module.c b/libdwfl/dwfl_segment_report_module.c
index 3ef62a7d..09ee37b3 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -737,17 +737,34 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name,
&& invalid_elf (module->elf, module->disk_file_has_build_id,
&build_id))
{
- elf_end (module->elf);
- close (module->fd);
- module->elf = NULL;
- module->fd = -1;
+ /* If MODULE's build-id doesn't match the disk file's
+ build-id, close ELF only if MODULE and ELF refer to
+ different builds of files with the same name. This
+ prevents premature closure of the correct ELF in cases
+ where segments of a module are non-contiguous in memory. */
+ if (name != NULL && module->name[0] != '\0'
+ && strcmp (basename (module->name), basename (name)) == 0)
+ {
+ elf_end (module->elf);
+ close (module->fd);
+ module->elf = NULL;
+ module->fd = -1;
+ }
}
- if (module->elf != NULL)
+ else if (module->elf != NULL)
{
- /* Ignore this found module if it would conflict in address
- space with any already existing module of DWFL. */
+ /* This module has already been reported. */
skip_this_module = true;
}
+ else
+ {
+ /* Only report this module if we haven't already done so. */
+ for (Dwfl_Module *mod = dwfl->modulelist; mod != NULL;
+ mod = mod->next)
+ if (mod->low_addr == module_start
+ && mod->high_addr == module_end)
+ skip_this_module = true;
+ }
}
if (skip_this_module)
goto out;
@@ -781,10 +798,6 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name,
}
}
- /* Our return value now says to skip the segments contained
- within the module. */
- ndx = addr_segndx (dwfl, segment, module_end, true);
-
/* Examine its .dynamic section to get more interesting details.
If it has DT_SONAME, we'll use that as the module name.
If it has a DT_DEBUG, then it's actually a PIE rather than a DSO.
@@ -929,6 +942,8 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name,
ndx = -1;
goto out;
}
+ else
+ ndx++;
/* We have reported the module. Now let the caller decide whether we
should read the whole thing in right now. */
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 7fb8efb1..a12df1d3 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -212,7 +212,7 @@ TESTS = run-arextract.sh run-arsymtest.sh run-ar.sh newfile test-nlist \
$(asm_TESTS) run-disasm-bpf.sh run-low_high_pc-dw-form-indirect.sh \
run-nvidia-extended-linemap-libdw.sh run-nvidia-extended-linemap-readelf.sh \
run-readelf-dw-form-indirect.sh run-strip-largealign.sh \
- run-readelf-Dd.sh
+ run-readelf-Dd.sh run-unstrip-noncontig.sh
if !BIARCH
export ELFUTILS_DISABLE_BIARCH = 1
@@ -632,7 +632,8 @@ EXTRA_DIST = run-arextract.sh run-arsymtest.sh run-ar.sh \
run-nvidia-extended-linemap-libdw.sh run-nvidia-extended-linemap-readelf.sh \
testfile_nvidia_linemap.bz2 \
testfile-largealign.o.bz2 run-strip-largealign.sh \
- run-funcretval++11.sh
+ run-funcretval++11.sh \
+ run-unstrip-noncontig.sh testcore-noncontig.bz2
if USE_VALGRIND
diff --git a/tests/run-unstrip-noncontig.sh b/tests/run-unstrip-noncontig.sh
new file mode 100755
index 00000000..37617272
--- /dev/null
+++ b/tests/run-unstrip-noncontig.sh
@@ -0,0 +1,155 @@
+#! /bin/sh
+# Copyright (C) 2023 Red Hat, 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
+
+# Test whether libdwfl can handle corefiles containing non-contiguous
+# segments where multiple modules are contained within the address
+# space of some other module.
+
+# testcore-noncontig is a corefile of a `firefox --headless` process.
+# testcore-noncontig was generated on systemd-coredump on RHEL 7.9
+# updates-20231031.1 with Firefox version 115.4.0esr
+
+testfiles testcore-noncontig
+tempfiles out
+
+# Remove parts of the output that could change depending on which
+# libraries are locally installed.
+testrun ${abs_top_builddir}/src/unstrip -n --core testcore-noncontig \
+ | sed 's/+/ /g' | cut -d " " -f1,3 | sort > out
+
+testrun_compare cat out <<\EOF
+0x563c47849000 7fed8657c7087c4e8dd4aa982c80c25cd2e2184f@0x563c4784930c
+0x7f09200af000 1b56e03a022f78b896d76024562aab78f1ade672@0x7f09200af1d8
+0x7f09466f3000 23487a268085e6e9d475344c5c20b9311af6ab7c@0x7f09466f31d8
+0x7f0946aae000 9590277c7c5cdfbbf9ba08202f0460b617e9d743@0x7f0946aae1d8
+0x7f0946cb1000 7e4f82b215acdd26bbbba27aaba317cebf954813@0x7f0946cb11d8
+0x7f0946ec4000 c8e0a9ac88e923ae68a19e90641a37dbbce50bf6@0x7f0946ec41d8
+0x7f0947245000 62e8e26138d110bb0684fb69153f77d7ab8dd977@0x7f09472451d8
+0x7f09474f7000 616cff8d0672f4554dfce229942c29282d2165bb@0x7f09474f71d8
+0x7f09477ac000 36f8679ee9e43701c5358dfee83a1775f49a4d90@0x7f09477ac1d8
+0x7f0948f2f000 1986ad22e3d90b1a24e1132d1761dec7beab68ad@0x7f0948f2f1d8
+0x7f0949136000 7548d115412cc33bf67c1598e446c70daa1b7461@0x7f09491361d8
+0x7f0949363000 e54da1382c034ef216379710265df600eb741e6d@0x7f09493631d8
+0x7f0949832000 f86665bd3c509cd8077e418da3f481bb8844f886@0x7f09498321d8
+0x7f0949a77000 88e542e108877361947b9c99df2cc1a6c052fb9d@0x7f0949a771d8
+0x7f0949c83000 1dee227cb9622ed3621e03cc23d891d857ee1790@0x7f0949c831d8
+0x7f0949e9d000 07724eb0414130be5044af286665f291a4b84a17@0x7f0949e9d1d8
+0x7f094a0a3000 604ba93e18823ad1ee731841f9186c27501d4161@0x7f094a0a31d8
+0x7f094a302000 f751fef2271cc88c55becda85d9531c286bf5812@0x7f094a3021d8
+0x7f094a50d000 fdfcf8fba79b0117c8e1daf341b223e2ba926775@0x7f094a50d1d8
+0x7f094a715000 a0a18d4218077c8929de9bdbce169a0f545178f2@0x7f094a7151d8
+0x7f094a931000 1e6bb8f99e995f194c49edd175e0131d9db5a4b2@0x7f094a9311d8
+0x7f094abb1000 70b0d368880623fbdbc10f0801c86a6a94ef476b@0x7f094abb11d8
+0x7f0955008000 e85be5df704f175ddf23b652c6154b304aaa60ff@0x7f09550081d8
+0x7f095520f000 e745ee2423d5388da9345298b9499a5308e467b2@0x7f095520f1d8
+0x7f095b964000 627905339f4bef158c643a6690c88184e89b964c@0x7f095b9641d8
+0x7f095bb66000 3f3a9b2a16dcb99d3a0693dae3b4eb1d80eda6e7@0x7f095bb661d8
+0x7f095bd8e000 8be4ff424ef0474750663c1b28e70ef8034be15d@0x7f095bd8e1d8
+0x7f095bfbf000 42a08d09168d524c71547a60e4a8b20c478cc5b7@0x7f095bfbf1d8
+0x7f095c2f9000 a830f3f3565e47c6b5fbcc73cc7525325620204d@0x7f095c2f91d8
+0x7f095c521000 0e7cb0216617cc5ab67db0b544eb144dee6b8bd1@0x7f095c5211d8
+0x7f095c786000 859e73b7bcd2d7e79f6eb89115f70ca8a9454e92@0x7f095c7861d8
+0x7f095c98c000 642c7bac832a1030e538c4de2d49f69cf68896ea@0x7f095c98c210
+0x7f095cc8c000 f8faf2d3317762a737efe1effd4cc72381616110@0x7f095cc8c280
+0x7f0966184000 72ff6c18bf2c3694c28aa74e79a96a7e3cfebef1@0x7f0966184210
+0x7f096639c000 3f24c8e8e78bc5f4f66aa11eccf86991a6c777eb@0x7f096639c1d8
+0x7f09665a1000 70b9cf3e05aef24b2cc397b01f7f99552a997290@0x7f09665a1210
+0x7f09667f2000 d9a4e213242676953a0ed8894917ecee2620d07a@0x7f09667f21d8
+0x7f09669f7000 d1a311a30d1f4f08e667931a89fa504d44434d51@0x7f09669f71d8
+0x7f0966c78000 1227a4fb1f504b57bbaea7de773c5d7b3fdfef67@0x7f0966c781d8
+0x7f0966e87000 f744796ea6fa4d8019414f5487ee85de74dc5adc@0x7f0966e871d8
+0x7f09670ad000 3bc565e0565c33b1bd37ae0070f7d8e2ce4313e4@0x7f09670ad1d8
+0x7f09672b2000 130ad6a89bad9508721c9c65f05b6073aefab954@0x7f09672b2210
+0x7f09674f2000 64abb184dea0eab3c97cac8ff2a0f908c88ec00f@0x7f09674f21d8
+0x7f0967720000 69c12493186fc344e336a1a90d0f97358597c8c7@0x7f0967720210
+0x7f0967951000 ff4b7273b270d970e174155b180689fbc0162aa1@0x7f09679511d8
+0x7f0967b83000 4763ea0f9967354676ba5d089f5a17980bf69d74@0x7f0967b83210
+0x7f0967e39000 ca7807fa2a9e0027b0fa5cc2ff4470bbc51ffefc@0x7f0967e391d8
+0x7f096803d000 e31fa8f847e049a7ab920ba0aad2113c080b2cc8@0x7f096803d1d8
+0x7f096824d000 2f1008b054be76260028b63c782bdfd3fc9fe667@0x7f096824d210
+0x7f0968490000 e0cd0dd5466e6b9e5fb10bfaff13b1bb50f08eaa@0x7f09684901d8
+0x7f09686aa000 805ab866a4573efec4d8ea95123e8349b2b9d349@0x7f09686aa210
+0x7f09688d1000 fb47c8b42bba8c045d274fff6c275d6e46c2140b@0x7f09688d1210
+0x7f0968ad6000 e7c193a97464ba51d738d50ed6ecb8011a73aea8@0x7f0968ad61d8
+0x7f0968d01000 c80bf962da9846b5f314dd459a8eefc8bcfcc057@0x7f0968d011d8
+0x7f0968f1d000 c2a5461fd0e7c4586c3fe708dd110c0191b0e039@0x7f0968f1d1d8
+0x7f0969129000 184f2d64788fe6d6b7b61aa4d4c40507d4ea7d68@0x7f09691291d8
+0x7f09693c6000 a564004324b7efeaa566e63abe279aa6b3594dc3@0x7f09693c61d8
+0x7f09695ce000 7655f3a4082ef7c50471ff3182c74a38ac071053@0x7f09695ce1d8
+0x7f096981e000 70b6d390923a83087b0159927169d158a4dddd41@0x7f096981e1d8
+0x7f0969a4f000 9c7316c5b03b098702e144fba8b81f2680b1bf6f@0x7f0969a4f1d8
+0x7f0969cdb000 8f3f245bd066e1a955da66a1546978dac0e2dce8@0x7f0969cdb1d8
+0x7f0969ef1000 6eb31a6023ba58f22e500ee180e308df74fdd717@0x7f0969ef11d8
+0x7f096a0fc000 889f480c11b989ed0c708bc2b0c398e873cbbfc8@0x7f096a0fc1d8
+0x7f096a30a000 ce9c7912583e5503d22c38f9fd26f907421ca2a6@0x7f096a30a1d8
+0x7f096a50e000 9d9cfc7df15ad65221aa780b48abb857d9ec7b1d@0x7f096a50e1d8
+0x7f096a739000 4935ec9bd3bb144f3a3b9df2e6b5e703709e70ea@0x7f096a7391d8
+0x7f096a94d000 677525c6c11af2fd3a807551fe550a3e53e562c1@0x7f096a94d210
+0x7f096abf6000 c26cfff2e8122580f5fbdab80b7805a1b8fce8af@0x7f096abf61d8
+0x7f096ae1e000 b0400174b1a3567df315c8e130f28dba8cb4ec0e@0x7f096ae1e1d8
+0x7f096b0dd000 f5b144f9f5d9be451c80211b34db2ce348e039b6@0x7f096b0dd1d8
+0x7f096b33f000 b41e62bbe61b9b4c6042d42c2f5af7d297144d8f@0x7f096b33f1d8
+0x7f096b551000 4cfa7c2a8f79f52aa132527f31d5287175309924@0x7f096b5511d8
+0x7f096b753000 1108b9ea7bc37c5143a29e06a9e6e4d277f42734@0x7f096b7531d8
+0x7f096b95b000 41579bc1b2fd7d382c7dcb726307823367c1819b@0x7f096b95b1d8
+0x7f096bb9b000 f2709bb30b8496f436775e13d2250d5565fa8b64@0x7f096bb9b1d8
+0x7f096bd9e000 d9f8562a7e670c65acf792e6ddc4ca4618a71447@0x7f096bd9e1d8
+0x7f096bfa1000 51286140acb14d6e4a4f1011b795b6c33336c884@0x7f096bfa11d8
+0x7f096c1ac000 cfe3e3e145bdd69171566516b9206ce1b25db0a6@0x7f096c1ac1d8
+0x7f096c3b7000 4372d2fdba1ce0f2ec0fa2827a18e79bbee69bc0@0x7f096c3b71d8
+0x7f096c5ba000 542038696c35b4e83607d94cf8189d0007fa4fcc@0x7f096c5ba1d8
+0x7f096c8d0000 1c7a65dd2934f5165cb9af16da26bad6a0ff01a0@0x7f096c8d01d8
+0x7f096cb21000 8d32e53efdf47c9c26d337dd1ee6a345f5e9bcdb@0x7f096cb211d8
+0x7f096cec1000 7f9a76fa3c45753dce18c42be9a10f4b037f6636@0x7f096cec11d8
+0x7f096d103000 c49e2c8aba746071fb43a90ec6c647139521f81e@0x7f096d1031d8
+0x7f096d349000 5daebcb97b7f2ee279f7888a9ba235f3ae2af28b@0x7f096d3491d8
+0x7f096d55f000 8c27e929990650fadaf88286147a07d199e9a9f9@0x7f096d55f1d8
+0x7f096d88b000 8cb793ddedc2563cc9ce294068d2b5592b99c5eb@0x7f096d88b1d8
+0x7f096da9a000 913feb13d07c401afaec5330acc0141f445ee089@0x7f096da9a1d8
+0x7f096dcc9000 e01bf260f55676dfee51152db1bb3286631f681d@0x7f096dcc91d8
+0x7f096deef000 ae9a9eadbb4df7ec86a6377d66c5bc2d3ff8077a@0x7f096deef1d8
+0x7f096e117000 81eac1acb73bc8017e918133dd266e6a556fbf8f@0x7f096e1171d8
+0x7f096e44e000 608a0d15fb6a196c15b19924f9b070417cab21c5@0x7f096e44e1d8
+0x7f096e657000 07062d2deb4d533956a51bcf82f487e2bcd14c11@0x7f096e6571d8
+0x7f096e85d000 523e8af0687380c9e1dfe118d133f5a1c3072b6f@0x7f096e85d1d8
+0x7f096ea6d000 f25c28bd5bce03ac2b33d03a6325eed3eeb38ae5@0x7f096ea6d1d8
+0x7f096edab000 b222556b5ef51f31c3cd15b3907b10a5e15503ba@0x7f096edab1d8
+0x7f096efb9000 c7a28bac87e0248d9bdcad67c0a49c16e4c94d58@0x7f096efb91d8
+0x7f096f1bd000 f2ba6aeb75e9ed7cbfd17a4f51464f60657a886e@0x7f096f1bd1d8
+0x7f096f4a7000 07c3de816e877f645438a333d20181004752cde7@0x7f096f4a71d8
+0x7f096fdaf000 3e44df7055942478d052e40fdd1f5b7862b152b0@0x7f096fdaf1d8
+0x7f096ffb7000 0c144eb085bbff4d20c65e5c3edb9b279a89a219@0x7f096ffb71d8
+0x7f09701f6000 9aa82814dd86874606ebbc1d24193309edd03c19@0x7f09701f61d8
+0x7f09703fb000 9e4e853011386be55252a0b9b762c21d9776585e@0x7f09703fb1d8
+0x7f0971074000 fc4fa58e47a5acc137eadb7689bce4357c557a96@0x7f0971074280
+0x7f0971442000 edf51350c7f71496149d064aa8b1441f786df88a@0x7f09714421d8
+0x7f0971658000 7615604eaf4a068dfae5085444d15c0dee93dfbd@0x7f09716581d8
+0x7f097195a000 09cfb171310110bc7ea9f4476c9fa044d85baff4@0x7f097195a210
+0x7f0971c62000 7f2e9cb0769d7e57bd669b485a74b537b63a57c4@0x7f0971c621d8
+0x7f0971e66000 e10cc8f2b932fc3daeda22f8dac5ebb969524e5b@0x7f0971e66248
+0x7f0972082000 62c449974331341bb08dcce3859560a22af1e172@0x7f09720821d8
+0x7f0972134000 ea08a2a179bfeb848ec18b4cefdc13bad833cde2@0x7f0972134248
+0x7f097213a000 d16c5befe2d75056c0bcc7a753575dd4cc53d5a8@0x7f097213a248
+0x7f0972250000 175efdcef445455872a86a6fbee7567ca16a513e@0x7f0972250248
+0x7f097225c000 64e373a32acff49f556ab8ab188f16ef418bd734@0x7f097225c248
+0x7f097229c000 bb811203073d0b6cba5c5e95b5a1e997c01d1e00@0x7f097229c248
+0x7ffea8bc9000 94937c5ca177227f631c6a53fa3a7b8ba1f21507@0x7ffea8bc9328
+EOF
+
+exit 0
diff --git a/tests/testcore-noncontig.bz2 b/tests/testcore-noncontig.bz2
new file mode 100644
index 0000000000000000000000000000000000000000..56e931d10032dffb239a5b0fba39b2abadf1b944
GIT binary patch
[Very large binary snipped]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] libdwfl: Correctly handle corefile non-contiguous segments
2023-11-12 20:16 [PATCH] libdwfl: Correctly handle corefile non-contiguous segments Aaron Merey
@ 2023-11-13 14:44 ` Aaron Merey
2023-11-14 14:03 ` Mark Wielaard
1 sibling, 0 replies; 4+ messages in thread
From: Aaron Merey @ 2023-11-13 14:44 UTC (permalink / raw)
To: elfutils-devel
BTW this patch won't apply as-is. I removed the firefox corefile since
I'd rather not send a >50M email to this list :)
See branch users/amerey/try-pr30975 if you want to run the code.
Aaron
On Sun, Nov 12, 2023 at 3:16 PM Aaron Merey <amerey@redhat.com> wrote:
>
> It is possible for segments of different shared libaries to be interleaved
> in memory such that the segments of one library are located in between
> non-contiguous segments of another library.
>
> For example, this can be seen with firefox on RHEL 7.9 where multiple
> shared libraries could be mapped in between ld-2.17.so segments:
>
> [...]
> 7f0972082000-7f09720a4000 00000000 139264 /usr/lib64/ld-2.17.so
> 7f09720a4000-7f09720a5000 00000000 4096 /memfd:mozilla-ipc (deleted)
> 7f09720a5000-7f09720a7000 00000000 8192 /memfd:mozilla-ipc (deleted)
> 7f09720a7000-7f09720a9000 00000000 8192 /memfd:mozilla-ipc (deleted)
> 7f0972134000-7f0972136000 00000000 8192 /usr/lib64/firefox/libmozwayland.so
> 7f0972136000-7f0972137000 00002000 4096 /usr/lib64/firefox/libmozwayland.so
> 7f0972137000-7f0972138000 00003000 4096 /usr/lib64/firefox/libmozwayland.so
> 7f0972138000-7f0972139000 00003000 4096 /usr/lib64/firefox/libmozwayland.so
> 7f097213a000-7f0972147000 00000000 53248 /usr/lib64/firefox/libmozsqlite3.so
> 7f0972147000-7f097221e000 0000d000 880640 /usr/lib64/firefox/libmozsqlite3.so
> 7f097221e000-7f0972248000 000e4000 172032 /usr/lib64/firefox/libmozsqlite3.so
> 7f0972248000-7f0972249000 0010e000 4096 /usr/lib64/firefox/libmozsqlite3.so
> 7f0972249000-7f097224c000 0010e000 12288 /usr/lib64/firefox/libmozsqlite3.so
> 7f097224c000-7f0972250000 00111000 16384 /usr/lib64/firefox/libmozsqlite3.so
> 7f0972250000-7f0972253000 00000000 12288 /usr/lib64/firefox/liblgpllibs.so
> [...]
> 7f09722a3000-7f09722a4000 00021000 4096 /usr/lib64/ld-2.17.so
> 7f09722a4000-7f09722a5000 00022000 4096 /usr/lib64/ld-2.17.so
>
> dwfl_segment_report_module did not account for the possibility of
> interleaving non-contiguous segments, resulting in premature closure
> of modules as well as failing to report modules.
>
> Fix this by removing segment skipping in dwfl_segment_report_module.
> When dwfl_segment_report_module reported a module, it would return
> the index of the segment immediately following the end address of the
> current module. Since there's a chance that other modules might fall
> within this address range, dwfl_segment_report_module instead returns
> the index of the next segment.
>
> This patch also fixes premature module closure that can occur in
> dwfl_segment_report_module when interleaving non-contiguous segments
> are found. Previously modules with start and end addresses that overlap
> with the current segment would have their build-id compared the build-id
> associated with the current segment. If there was a mismatch, that module
> would be closed. Avoid closing modules in this case when mismatching
> build-ids correspond to distinct modules.
>
> A couple caveats should be mentioned. First, start and end addresses
> of reported modules cannot be assumed to contain segments from only
> that module. This has always been the case however.
>
> Second, the testcases in this patch use a firefox corefile that is
> fairly large. The .bz2 corefile is about 47M. A clean elfutils repo
> is currently about 42M, so this corefile more than doubles the size of
> the elfutils repo. I looked for a much smaller process with
> interleaving non-contiguous shared library sections but was not able
> to find one. I've included the corefile and tests in this patch but
> they can be removed if we'd prefer to not approx. double the size of
> the repo.
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=30975
>
> Signed-off-by: Aaron Merey <amerey@redhat.com>
> ---
> libdwfl/dwfl_segment_report_module.c | 37 +++++--
> tests/Makefile.am | 5 +-
> tests/run-unstrip-noncontig.sh | 155 +++++++++++++++++++++++++++
> tests/testcore-noncontig.bz2 | Bin 0 -> 49146091 bytes
> 4 files changed, 184 insertions(+), 13 deletions(-)
> create mode 100755 tests/run-unstrip-noncontig.sh
> create mode 100644 tests/testcore-noncontig.bz2
>
> diff --git a/libdwfl/dwfl_segment_report_module.c b/libdwfl/dwfl_segment_report_module.c
> index 3ef62a7d..09ee37b3 100644
> --- a/libdwfl/dwfl_segment_report_module.c
> +++ b/libdwfl/dwfl_segment_report_module.c
> @@ -737,17 +737,34 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name,
> && invalid_elf (module->elf, module->disk_file_has_build_id,
> &build_id))
> {
> - elf_end (module->elf);
> - close (module->fd);
> - module->elf = NULL;
> - module->fd = -1;
> + /* If MODULE's build-id doesn't match the disk file's
> + build-id, close ELF only if MODULE and ELF refer to
> + different builds of files with the same name. This
> + prevents premature closure of the correct ELF in cases
> + where segments of a module are non-contiguous in memory. */
> + if (name != NULL && module->name[0] != '\0'
> + && strcmp (basename (module->name), basename (name)) == 0)
> + {
> + elf_end (module->elf);
> + close (module->fd);
> + module->elf = NULL;
> + module->fd = -1;
> + }
> }
> - if (module->elf != NULL)
> + else if (module->elf != NULL)
> {
> - /* Ignore this found module if it would conflict in address
> - space with any already existing module of DWFL. */
> + /* This module has already been reported. */
> skip_this_module = true;
> }
> + else
> + {
> + /* Only report this module if we haven't already done so. */
> + for (Dwfl_Module *mod = dwfl->modulelist; mod != NULL;
> + mod = mod->next)
> + if (mod->low_addr == module_start
> + && mod->high_addr == module_end)
> + skip_this_module = true;
> + }
> }
> if (skip_this_module)
> goto out;
> @@ -781,10 +798,6 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name,
> }
> }
>
> - /* Our return value now says to skip the segments contained
> - within the module. */
> - ndx = addr_segndx (dwfl, segment, module_end, true);
> -
> /* Examine its .dynamic section to get more interesting details.
> If it has DT_SONAME, we'll use that as the module name.
> If it has a DT_DEBUG, then it's actually a PIE rather than a DSO.
> @@ -929,6 +942,8 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name,
> ndx = -1;
> goto out;
> }
> + else
> + ndx++;
>
> /* We have reported the module. Now let the caller decide whether we
> should read the whole thing in right now. */
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 7fb8efb1..a12df1d3 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -212,7 +212,7 @@ TESTS = run-arextract.sh run-arsymtest.sh run-ar.sh newfile test-nlist \
> $(asm_TESTS) run-disasm-bpf.sh run-low_high_pc-dw-form-indirect.sh \
> run-nvidia-extended-linemap-libdw.sh run-nvidia-extended-linemap-readelf.sh \
> run-readelf-dw-form-indirect.sh run-strip-largealign.sh \
> - run-readelf-Dd.sh
> + run-readelf-Dd.sh run-unstrip-noncontig.sh
>
> if !BIARCH
> export ELFUTILS_DISABLE_BIARCH = 1
> @@ -632,7 +632,8 @@ EXTRA_DIST = run-arextract.sh run-arsymtest.sh run-ar.sh \
> run-nvidia-extended-linemap-libdw.sh run-nvidia-extended-linemap-readelf.sh \
> testfile_nvidia_linemap.bz2 \
> testfile-largealign.o.bz2 run-strip-largealign.sh \
> - run-funcretval++11.sh
> + run-funcretval++11.sh \
> + run-unstrip-noncontig.sh testcore-noncontig.bz2
>
>
> if USE_VALGRIND
> diff --git a/tests/run-unstrip-noncontig.sh b/tests/run-unstrip-noncontig.sh
> new file mode 100755
> index 00000000..37617272
> --- /dev/null
> +++ b/tests/run-unstrip-noncontig.sh
> @@ -0,0 +1,155 @@
> +#! /bin/sh
> +# Copyright (C) 2023 Red Hat, 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
> +
> +# Test whether libdwfl can handle corefiles containing non-contiguous
> +# segments where multiple modules are contained within the address
> +# space of some other module.
> +
> +# testcore-noncontig is a corefile of a `firefox --headless` process.
> +# testcore-noncontig was generated on systemd-coredump on RHEL 7.9
> +# updates-20231031.1 with Firefox version 115.4.0esr
> +
> +testfiles testcore-noncontig
> +tempfiles out
> +
> +# Remove parts of the output that could change depending on which
> +# libraries are locally installed.
> +testrun ${abs_top_builddir}/src/unstrip -n --core testcore-noncontig \
> + | sed 's/+/ /g' | cut -d " " -f1,3 | sort > out
> +
> +testrun_compare cat out <<\EOF
> +0x563c47849000 7fed8657c7087c4e8dd4aa982c80c25cd2e2184f@0x563c4784930c
> +0x7f09200af000 1b56e03a022f78b896d76024562aab78f1ade672@0x7f09200af1d8
> +0x7f09466f3000 23487a268085e6e9d475344c5c20b9311af6ab7c@0x7f09466f31d8
> +0x7f0946aae000 9590277c7c5cdfbbf9ba08202f0460b617e9d743@0x7f0946aae1d8
> +0x7f0946cb1000 7e4f82b215acdd26bbbba27aaba317cebf954813@0x7f0946cb11d8
> +0x7f0946ec4000 c8e0a9ac88e923ae68a19e90641a37dbbce50bf6@0x7f0946ec41d8
> +0x7f0947245000 62e8e26138d110bb0684fb69153f77d7ab8dd977@0x7f09472451d8
> +0x7f09474f7000 616cff8d0672f4554dfce229942c29282d2165bb@0x7f09474f71d8
> +0x7f09477ac000 36f8679ee9e43701c5358dfee83a1775f49a4d90@0x7f09477ac1d8
> +0x7f0948f2f000 1986ad22e3d90b1a24e1132d1761dec7beab68ad@0x7f0948f2f1d8
> +0x7f0949136000 7548d115412cc33bf67c1598e446c70daa1b7461@0x7f09491361d8
> +0x7f0949363000 e54da1382c034ef216379710265df600eb741e6d@0x7f09493631d8
> +0x7f0949832000 f86665bd3c509cd8077e418da3f481bb8844f886@0x7f09498321d8
> +0x7f0949a77000 88e542e108877361947b9c99df2cc1a6c052fb9d@0x7f0949a771d8
> +0x7f0949c83000 1dee227cb9622ed3621e03cc23d891d857ee1790@0x7f0949c831d8
> +0x7f0949e9d000 07724eb0414130be5044af286665f291a4b84a17@0x7f0949e9d1d8
> +0x7f094a0a3000 604ba93e18823ad1ee731841f9186c27501d4161@0x7f094a0a31d8
> +0x7f094a302000 f751fef2271cc88c55becda85d9531c286bf5812@0x7f094a3021d8
> +0x7f094a50d000 fdfcf8fba79b0117c8e1daf341b223e2ba926775@0x7f094a50d1d8
> +0x7f094a715000 a0a18d4218077c8929de9bdbce169a0f545178f2@0x7f094a7151d8
> +0x7f094a931000 1e6bb8f99e995f194c49edd175e0131d9db5a4b2@0x7f094a9311d8
> +0x7f094abb1000 70b0d368880623fbdbc10f0801c86a6a94ef476b@0x7f094abb11d8
> +0x7f0955008000 e85be5df704f175ddf23b652c6154b304aaa60ff@0x7f09550081d8
> +0x7f095520f000 e745ee2423d5388da9345298b9499a5308e467b2@0x7f095520f1d8
> +0x7f095b964000 627905339f4bef158c643a6690c88184e89b964c@0x7f095b9641d8
> +0x7f095bb66000 3f3a9b2a16dcb99d3a0693dae3b4eb1d80eda6e7@0x7f095bb661d8
> +0x7f095bd8e000 8be4ff424ef0474750663c1b28e70ef8034be15d@0x7f095bd8e1d8
> +0x7f095bfbf000 42a08d09168d524c71547a60e4a8b20c478cc5b7@0x7f095bfbf1d8
> +0x7f095c2f9000 a830f3f3565e47c6b5fbcc73cc7525325620204d@0x7f095c2f91d8
> +0x7f095c521000 0e7cb0216617cc5ab67db0b544eb144dee6b8bd1@0x7f095c5211d8
> +0x7f095c786000 859e73b7bcd2d7e79f6eb89115f70ca8a9454e92@0x7f095c7861d8
> +0x7f095c98c000 642c7bac832a1030e538c4de2d49f69cf68896ea@0x7f095c98c210
> +0x7f095cc8c000 f8faf2d3317762a737efe1effd4cc72381616110@0x7f095cc8c280
> +0x7f0966184000 72ff6c18bf2c3694c28aa74e79a96a7e3cfebef1@0x7f0966184210
> +0x7f096639c000 3f24c8e8e78bc5f4f66aa11eccf86991a6c777eb@0x7f096639c1d8
> +0x7f09665a1000 70b9cf3e05aef24b2cc397b01f7f99552a997290@0x7f09665a1210
> +0x7f09667f2000 d9a4e213242676953a0ed8894917ecee2620d07a@0x7f09667f21d8
> +0x7f09669f7000 d1a311a30d1f4f08e667931a89fa504d44434d51@0x7f09669f71d8
> +0x7f0966c78000 1227a4fb1f504b57bbaea7de773c5d7b3fdfef67@0x7f0966c781d8
> +0x7f0966e87000 f744796ea6fa4d8019414f5487ee85de74dc5adc@0x7f0966e871d8
> +0x7f09670ad000 3bc565e0565c33b1bd37ae0070f7d8e2ce4313e4@0x7f09670ad1d8
> +0x7f09672b2000 130ad6a89bad9508721c9c65f05b6073aefab954@0x7f09672b2210
> +0x7f09674f2000 64abb184dea0eab3c97cac8ff2a0f908c88ec00f@0x7f09674f21d8
> +0x7f0967720000 69c12493186fc344e336a1a90d0f97358597c8c7@0x7f0967720210
> +0x7f0967951000 ff4b7273b270d970e174155b180689fbc0162aa1@0x7f09679511d8
> +0x7f0967b83000 4763ea0f9967354676ba5d089f5a17980bf69d74@0x7f0967b83210
> +0x7f0967e39000 ca7807fa2a9e0027b0fa5cc2ff4470bbc51ffefc@0x7f0967e391d8
> +0x7f096803d000 e31fa8f847e049a7ab920ba0aad2113c080b2cc8@0x7f096803d1d8
> +0x7f096824d000 2f1008b054be76260028b63c782bdfd3fc9fe667@0x7f096824d210
> +0x7f0968490000 e0cd0dd5466e6b9e5fb10bfaff13b1bb50f08eaa@0x7f09684901d8
> +0x7f09686aa000 805ab866a4573efec4d8ea95123e8349b2b9d349@0x7f09686aa210
> +0x7f09688d1000 fb47c8b42bba8c045d274fff6c275d6e46c2140b@0x7f09688d1210
> +0x7f0968ad6000 e7c193a97464ba51d738d50ed6ecb8011a73aea8@0x7f0968ad61d8
> +0x7f0968d01000 c80bf962da9846b5f314dd459a8eefc8bcfcc057@0x7f0968d011d8
> +0x7f0968f1d000 c2a5461fd0e7c4586c3fe708dd110c0191b0e039@0x7f0968f1d1d8
> +0x7f0969129000 184f2d64788fe6d6b7b61aa4d4c40507d4ea7d68@0x7f09691291d8
> +0x7f09693c6000 a564004324b7efeaa566e63abe279aa6b3594dc3@0x7f09693c61d8
> +0x7f09695ce000 7655f3a4082ef7c50471ff3182c74a38ac071053@0x7f09695ce1d8
> +0x7f096981e000 70b6d390923a83087b0159927169d158a4dddd41@0x7f096981e1d8
> +0x7f0969a4f000 9c7316c5b03b098702e144fba8b81f2680b1bf6f@0x7f0969a4f1d8
> +0x7f0969cdb000 8f3f245bd066e1a955da66a1546978dac0e2dce8@0x7f0969cdb1d8
> +0x7f0969ef1000 6eb31a6023ba58f22e500ee180e308df74fdd717@0x7f0969ef11d8
> +0x7f096a0fc000 889f480c11b989ed0c708bc2b0c398e873cbbfc8@0x7f096a0fc1d8
> +0x7f096a30a000 ce9c7912583e5503d22c38f9fd26f907421ca2a6@0x7f096a30a1d8
> +0x7f096a50e000 9d9cfc7df15ad65221aa780b48abb857d9ec7b1d@0x7f096a50e1d8
> +0x7f096a739000 4935ec9bd3bb144f3a3b9df2e6b5e703709e70ea@0x7f096a7391d8
> +0x7f096a94d000 677525c6c11af2fd3a807551fe550a3e53e562c1@0x7f096a94d210
> +0x7f096abf6000 c26cfff2e8122580f5fbdab80b7805a1b8fce8af@0x7f096abf61d8
> +0x7f096ae1e000 b0400174b1a3567df315c8e130f28dba8cb4ec0e@0x7f096ae1e1d8
> +0x7f096b0dd000 f5b144f9f5d9be451c80211b34db2ce348e039b6@0x7f096b0dd1d8
> +0x7f096b33f000 b41e62bbe61b9b4c6042d42c2f5af7d297144d8f@0x7f096b33f1d8
> +0x7f096b551000 4cfa7c2a8f79f52aa132527f31d5287175309924@0x7f096b5511d8
> +0x7f096b753000 1108b9ea7bc37c5143a29e06a9e6e4d277f42734@0x7f096b7531d8
> +0x7f096b95b000 41579bc1b2fd7d382c7dcb726307823367c1819b@0x7f096b95b1d8
> +0x7f096bb9b000 f2709bb30b8496f436775e13d2250d5565fa8b64@0x7f096bb9b1d8
> +0x7f096bd9e000 d9f8562a7e670c65acf792e6ddc4ca4618a71447@0x7f096bd9e1d8
> +0x7f096bfa1000 51286140acb14d6e4a4f1011b795b6c33336c884@0x7f096bfa11d8
> +0x7f096c1ac000 cfe3e3e145bdd69171566516b9206ce1b25db0a6@0x7f096c1ac1d8
> +0x7f096c3b7000 4372d2fdba1ce0f2ec0fa2827a18e79bbee69bc0@0x7f096c3b71d8
> +0x7f096c5ba000 542038696c35b4e83607d94cf8189d0007fa4fcc@0x7f096c5ba1d8
> +0x7f096c8d0000 1c7a65dd2934f5165cb9af16da26bad6a0ff01a0@0x7f096c8d01d8
> +0x7f096cb21000 8d32e53efdf47c9c26d337dd1ee6a345f5e9bcdb@0x7f096cb211d8
> +0x7f096cec1000 7f9a76fa3c45753dce18c42be9a10f4b037f6636@0x7f096cec11d8
> +0x7f096d103000 c49e2c8aba746071fb43a90ec6c647139521f81e@0x7f096d1031d8
> +0x7f096d349000 5daebcb97b7f2ee279f7888a9ba235f3ae2af28b@0x7f096d3491d8
> +0x7f096d55f000 8c27e929990650fadaf88286147a07d199e9a9f9@0x7f096d55f1d8
> +0x7f096d88b000 8cb793ddedc2563cc9ce294068d2b5592b99c5eb@0x7f096d88b1d8
> +0x7f096da9a000 913feb13d07c401afaec5330acc0141f445ee089@0x7f096da9a1d8
> +0x7f096dcc9000 e01bf260f55676dfee51152db1bb3286631f681d@0x7f096dcc91d8
> +0x7f096deef000 ae9a9eadbb4df7ec86a6377d66c5bc2d3ff8077a@0x7f096deef1d8
> +0x7f096e117000 81eac1acb73bc8017e918133dd266e6a556fbf8f@0x7f096e1171d8
> +0x7f096e44e000 608a0d15fb6a196c15b19924f9b070417cab21c5@0x7f096e44e1d8
> +0x7f096e657000 07062d2deb4d533956a51bcf82f487e2bcd14c11@0x7f096e6571d8
> +0x7f096e85d000 523e8af0687380c9e1dfe118d133f5a1c3072b6f@0x7f096e85d1d8
> +0x7f096ea6d000 f25c28bd5bce03ac2b33d03a6325eed3eeb38ae5@0x7f096ea6d1d8
> +0x7f096edab000 b222556b5ef51f31c3cd15b3907b10a5e15503ba@0x7f096edab1d8
> +0x7f096efb9000 c7a28bac87e0248d9bdcad67c0a49c16e4c94d58@0x7f096efb91d8
> +0x7f096f1bd000 f2ba6aeb75e9ed7cbfd17a4f51464f60657a886e@0x7f096f1bd1d8
> +0x7f096f4a7000 07c3de816e877f645438a333d20181004752cde7@0x7f096f4a71d8
> +0x7f096fdaf000 3e44df7055942478d052e40fdd1f5b7862b152b0@0x7f096fdaf1d8
> +0x7f096ffb7000 0c144eb085bbff4d20c65e5c3edb9b279a89a219@0x7f096ffb71d8
> +0x7f09701f6000 9aa82814dd86874606ebbc1d24193309edd03c19@0x7f09701f61d8
> +0x7f09703fb000 9e4e853011386be55252a0b9b762c21d9776585e@0x7f09703fb1d8
> +0x7f0971074000 fc4fa58e47a5acc137eadb7689bce4357c557a96@0x7f0971074280
> +0x7f0971442000 edf51350c7f71496149d064aa8b1441f786df88a@0x7f09714421d8
> +0x7f0971658000 7615604eaf4a068dfae5085444d15c0dee93dfbd@0x7f09716581d8
> +0x7f097195a000 09cfb171310110bc7ea9f4476c9fa044d85baff4@0x7f097195a210
> +0x7f0971c62000 7f2e9cb0769d7e57bd669b485a74b537b63a57c4@0x7f0971c621d8
> +0x7f0971e66000 e10cc8f2b932fc3daeda22f8dac5ebb969524e5b@0x7f0971e66248
> +0x7f0972082000 62c449974331341bb08dcce3859560a22af1e172@0x7f09720821d8
> +0x7f0972134000 ea08a2a179bfeb848ec18b4cefdc13bad833cde2@0x7f0972134248
> +0x7f097213a000 d16c5befe2d75056c0bcc7a753575dd4cc53d5a8@0x7f097213a248
> +0x7f0972250000 175efdcef445455872a86a6fbee7567ca16a513e@0x7f0972250248
> +0x7f097225c000 64e373a32acff49f556ab8ab188f16ef418bd734@0x7f097225c248
> +0x7f097229c000 bb811203073d0b6cba5c5e95b5a1e997c01d1e00@0x7f097229c248
> +0x7ffea8bc9000 94937c5ca177227f631c6a53fa3a7b8ba1f21507@0x7ffea8bc9328
> +EOF
> +
> +exit 0
> diff --git a/tests/testcore-noncontig.bz2 b/tests/testcore-noncontig.bz2
> new file mode 100644
> index 0000000000000000000000000000000000000000..56e931d10032dffb239a5b0fba39b2abadf1b944
> GIT binary patch
> [Very large binary snipped]
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] libdwfl: Correctly handle corefile non-contiguous segments
2023-11-12 20:16 [PATCH] libdwfl: Correctly handle corefile non-contiguous segments Aaron Merey
2023-11-13 14:44 ` Aaron Merey
@ 2023-11-14 14:03 ` Mark Wielaard
2023-11-14 18:46 ` Aaron Merey
1 sibling, 1 reply; 4+ messages in thread
From: Mark Wielaard @ 2023-11-14 14:03 UTC (permalink / raw)
To: Aaron Merey, elfutils-devel
Hi Aaron,
On Sun, 2023-11-12 at 15:16 -0500, Aaron Merey wrote:
> It is possible for segments of different shared libaries to be interleaved
> in memory such that the segments of one library are located in between
> non-contiguous segments of another library.
>
> For example, this can be seen with firefox on RHEL 7.9 where multiple
> shared libraries could be mapped in between ld-2.17.so segments:
>
> [...]
> 7f0972082000-7f09720a4000 00000000 139264 /usr/lib64/ld-2.17.so
> 7f09720a4000-7f09720a5000 00000000 4096 /memfd:mozilla-ipc (deleted)
> 7f09720a5000-7f09720a7000 00000000 8192 /memfd:mozilla-ipc (deleted)
> 7f09720a7000-7f09720a9000 00000000 8192 /memfd:mozilla-ipc (deleted)
> 7f0972134000-7f0972136000 00000000 8192 /usr/lib64/firefox/libmozwayland.so
> 7f0972136000-7f0972137000 00002000 4096 /usr/lib64/firefox/libmozwayland.so
> 7f0972137000-7f0972138000 00003000 4096 /usr/lib64/firefox/libmozwayland.so
> 7f0972138000-7f0972139000 00003000 4096 /usr/lib64/firefox/libmozwayland.so
> 7f097213a000-7f0972147000 00000000 53248 /usr/lib64/firefox/libmozsqlite3.so
> 7f0972147000-7f097221e000 0000d000 880640 /usr/lib64/firefox/libmozsqlite3.so
> 7f097221e000-7f0972248000 000e4000 172032 /usr/lib64/firefox/libmozsqlite3.so
> 7f0972248000-7f0972249000 0010e000 4096 /usr/lib64/firefox/libmozsqlite3.so
> 7f0972249000-7f097224c000 0010e000 12288 /usr/lib64/firefox/libmozsqlite3.so
> 7f097224c000-7f0972250000 00111000 16384 /usr/lib64/firefox/libmozsqlite3.so
> 7f0972250000-7f0972253000 00000000 12288 /usr/lib64/firefox/liblgpllibs.so
> [...]
> 7f09722a3000-7f09722a4000 00021000 4096 /usr/lib64/ld-2.17.so
> 7f09722a4000-7f09722a5000 00022000 4096 /usr/lib64/ld-2.17.so
>
> dwfl_segment_report_module did not account for the possibility of
> interleaving non-contiguous segments, resulting in premature closure
> of modules as well as failing to report modules.
Nice description of the issue.
> Fix this by removing segment skipping in dwfl_segment_report_module.
> When dwfl_segment_report_module reported a module, it would return
> the index of the segment immediately following the end address of the
> current module. Since there's a chance that other modules might fall
> within this address range, dwfl_segment_report_module instead returns
> the index of the next segment.
This makes sense.
> This patch also fixes premature module closure that can occur in
> dwfl_segment_report_module when interleaving non-contiguous segments
> are found. Previously modules with start and end addresses that overlap
> with the current segment would have their build-id compared the build-id
> associated with the current segment. If there was a mismatch, that module
> would be closed. Avoid closing modules in this case when mismatching
> build-ids correspond to distinct modules.
Nice find.
> A couple caveats should be mentioned. First, start and end addresses
> of reported modules cannot be assumed to contain segments from only
> that module. This has always been the case however.
There is dwfl_addrmodule/dwfl_addrsegment to find the module that
covers a specific address. Defined in libdwfl/segment.c. I think this
should handle this by checking the closes load address. But I have not
tested it.
Normally only kernel modules (.ko ET_REL files) have multiple segments.
So it might make sense to double check none of this impacts systemtap.
> Second, the testcases in this patch use a firefox corefile that is
> fairly large. The .bz2 corefile is about 47M. A clean elfutils repo
> is currently about 42M, so this corefile more than doubles the size of
> the elfutils repo. I looked for a much smaller process with
> interleaving non-contiguous shared library sections but was not able
> to find one. I've included the corefile and tests in this patch but
> they can be removed if we'd prefer to not approx. double the size of
> the repo.
I really appreciate the testcase, but it really is too big. It is 736M
bunzip2ed (which would happen on any make check). I think this makes
the repo and the build/check a little too heavy. Also we try to include
instructions to recreate any binary test files and that isn't really
possible in this case.
> https://sourceware.org/bugzilla/show_bug.cgi?id=30975
>
> Signed-off-by: Aaron Merey <amerey@redhat.com>
> ---
> libdwfl/dwfl_segment_report_module.c | 37 +++++--
> tests/Makefile.am | 5 +-
> tests/run-unstrip-noncontig.sh | 155 +++++++++++++++++++++++++++
> tests/testcore-noncontig.bz2 | Bin 0 -> 49146091 bytes
> 4 files changed, 184 insertions(+), 13 deletions(-)
> create mode 100755 tests/run-unstrip-noncontig.sh
> create mode 100644 tests/testcore-noncontig.bz2
>
> diff --git a/libdwfl/dwfl_segment_report_module.c b/libdwfl/dwfl_segment_report_module.c
> index 3ef62a7d..09ee37b3 100644
> --- a/libdwfl/dwfl_segment_report_module.c
> +++ b/libdwfl/dwfl_segment_report_module.c
> @@ -737,17 +737,34 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name,
> && invalid_elf (module->elf, module->disk_file_has_build_id,
> &build_id))
> {
> - elf_end (module->elf);
> - close (module->fd);
> - module->elf = NULL;
> - module->fd = -1;
> + /* If MODULE's build-id doesn't match the disk file's
> + build-id, close ELF only if MODULE and ELF refer to
> + different builds of files with the same name. This
> + prevents premature closure of the correct ELF in cases
> + where segments of a module are non-contiguous in memory. */
> + if (name != NULL && module->name[0] != '\0'
> + && strcmp (basename (module->name), basename (name)) == 0)
> + {
> + elf_end (module->elf);
> + close (module->fd);
> + module->elf = NULL;
> + module->fd = -1;
> + }
> }
Nice.
> - if (module->elf != NULL)
> + else if (module->elf != NULL)
> {
> - /* Ignore this found module if it would conflict in address
> - space with any already existing module of DWFL. */
> + /* This module has already been reported. */
> skip_this_module = true;
> }
> + else
> + {
> + /* Only report this module if we haven't already done so. */
> + for (Dwfl_Module *mod = dwfl->modulelist; mod != NULL;
> + mod = mod->next)
> + if (mod->low_addr == module_start
> + && mod->high_addr == module_end)
> + skip_this_module = true;
> + }
> }
> if (skip_this_module)
> goto out;
OK. So now we only skip modules if we have already seen this exact
module.
> @@ -781,10 +798,6 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name,
> }
> }
>
> - /* Our return value now says to skip the segments contained
> - within the module. */
> - ndx = addr_segndx (dwfl, segment, module_end, true);
> -
> /* Examine its .dynamic section to get more interesting details.
> If it has DT_SONAME, we'll use that as the module name.
> If it has a DT_DEBUG, then it's actually a PIE rather than a DSO.
> @@ -929,6 +942,8 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name,
> ndx = -1;
> goto out;
> }
> + else
> + ndx++;
>
> /* We have reported the module. Now let the caller decide whether we
> should read the whole thing in right now. */
Right, so addr_segndx would normally return this ndx because next ==
true. But now we just always just report the next ndx.
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 7fb8efb1..a12df1d3 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -212,7 +212,7 @@ TESTS = run-arextract.sh run-arsymtest.sh run-ar.sh newfile test-nlist \
> $(asm_TESTS) run-disasm-bpf.sh run-low_high_pc-dw-form-indirect.sh \
> run-nvidia-extended-linemap-libdw.sh run-nvidia-extended-linemap-readelf.sh \
> run-readelf-dw-form-indirect.sh run-strip-largealign.sh \
> - run-readelf-Dd.sh
> + run-readelf-Dd.sh run-unstrip-noncontig.sh
>
> if !BIARCH
> export ELFUTILS_DISABLE_BIARCH = 1
> @@ -632,7 +632,8 @@ EXTRA_DIST = run-arextract.sh run-arsymtest.sh run-ar.sh \
> run-nvidia-extended-linemap-libdw.sh run-nvidia-extended-linemap-readelf.sh \
> testfile_nvidia_linemap.bz2 \
> testfile-largealign.o.bz2 run-strip-largealign.sh \
> - run-funcretval++11.sh
> + run-funcretval++11.sh \
> + run-unstrip-noncontig.sh testcore-noncontig.bz2
>
>
> if USE_VALGRIND
I really like to have a testcase, but only if we can find/construct
something much smaller.
Thanks,
Mark
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] libdwfl: Correctly handle corefile non-contiguous segments
2023-11-14 14:03 ` Mark Wielaard
@ 2023-11-14 18:46 ` Aaron Merey
0 siblings, 0 replies; 4+ messages in thread
From: Aaron Merey @ 2023-11-14 18:46 UTC (permalink / raw)
To: Mark Wielaard; +Cc: elfutils-devel
Hi Mark,
On Tue, Nov 14, 2023 at 9:03 AM Mark Wielaard <mark@klomp.org> wrote:
>
> > A couple caveats should be mentioned. First, start and end addresses
> > of reported modules cannot be assumed to contain segments from only
> > that module. This has always been the case however.
>
> There is dwfl_addrmodule/dwfl_addrsegment to find the module that
> covers a specific address. Defined in libdwfl/segment.c. I think this
> should handle this by checking the closes load address. But I have not
> tested it.
>
> Normally only kernel modules (.ko ET_REL files) have multiple segments.
> So it might make sense to double check none of this impacts systemtap.
I'll look into this.
> > Second, the testcases in this patch use a firefox corefile that is
> > fairly large. The .bz2 corefile is about 47M. A clean elfutils repo
> > is currently about 42M, so this corefile more than doubles the size of
> > the elfutils repo. I looked for a much smaller process with
> > interleaving non-contiguous shared library sections but was not able
> > to find one. I've included the corefile and tests in this patch but
> > they can be removed if we'd prefer to not approx. double the size of
> > the repo.
>
> I really appreciate the testcase, but it really is too big. It is 736M
> bunzip2ed (which would happen on any make check). I think this makes
> the repo and the build/check a little too heavy. Also we try to include
> instructions to recreate any binary test files and that isn't really
> possible in this case.
Agreed, this isn't a great testcase. I'm not sure if there's a way to
force the linker to create segments that reproduce the bug on a much
smaller process, but I'll see if I can write something by hand with
libelf. Or maybe GNU poke can be of use here? I'll take a look.
Aaron
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-11-14 18:46 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-12 20:16 [PATCH] libdwfl: Correctly handle corefile non-contiguous segments Aaron Merey
2023-11-13 14:44 ` Aaron Merey
2023-11-14 14:03 ` Mark Wielaard
2023-11-14 18:46 ` Aaron Merey
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).