On Tue, 2015-06-16 at 00:20 +0200, Mark Wielaard wrote: > Simplest probably is to just revert this one patch and do a elfutils > 0.163 release with just that (and the translation updates). The issue > that this was preventing (disk full with ELF_C_RDWR_MMAP, ftruncate > appear to succeed, write in memory map causes SIGBUS) is probably less > likely to trigger than this regression.
Even simpler is to always just (also) call ftruncate to set the right file size. Which is what the attached patch does. It also includes some test cases to check in-place-eu-strip actually reduces the file size. I'll update the elfutils fedora package so there is a fix right now. But unless people complain I would like to just make a 0.163 release end of the week with this fix and any other urgent fixes because I think this is a somewhat bad regression and we should urge people to not just patch, but to upgrade to a known good version. Thanks, Mark
From e4e846b67df12045ee6554bfb568a89b4ed80a71 Mon Sep 17 00:00:00 2001 From: Mark Wielaard <[email protected]> Date: Tue, 16 Jun 2015 14:05:35 +0200 Subject: [PATCH] libelf: Always call ftruncate before posix_fallocate to set the right size. When elf_update.c (write_file) doesn't know the current maximum file length it might have to reduce the file size. posix_fallocate can only extend the file. So always call ftruncate before that to set the file size and making sure the backing store is fully there. Add test cases for checking strip in place (eu-strip without -o) actually reduces the file size. But only for non-ET_REL files. We might not be able to strip ET_REL files (except when they are kernel modules) because they might contain "dangling" symbol table entries. https://bugzilla.redhat.com/show_bug.cgi?id=1232206 Signed-off-by: Mark Wielaard <[email protected]> --- libelf/ChangeLog | 5 +++++ libelf/elf_update.c | 7 +++++-- tests/ChangeLog | 5 +++++ tests/run-strip-test.sh | 13 +++++++++++++ 4 files changed, 28 insertions(+), 2 deletions(-) diff --git a/libelf/ChangeLog b/libelf/ChangeLog index 30017cd..2d24007 100644 --- a/libelf/ChangeLog +++ b/libelf/ChangeLog @@ -1,3 +1,8 @@ +2015-06-16 Mark Wielaard <[email protected]> + + * elf_update.c (write_file): Always also use ftruncate before + posix_fallocate to make sure file has the right size. + 2015-06-04 Mark Wielaard <[email protected]> * elf_getdata.c (__libelf_type_aligns): Add entries for ELF_T_EHDR, diff --git a/libelf/elf_update.c b/libelf/elf_update.c index 9e34c46..9eb007b 100644 --- a/libelf/elf_update.c +++ b/libelf/elf_update.c @@ -60,15 +60,18 @@ write_file (Elf *elf, off_t size, int change_bo, size_t shnum) new file. We truncate the file later in this case. Note we use posix_fallocate to make sure the file content is really - there. Using ftruncate might mean the file is extended, but space + there. Only using ftruncate might mean the file is extended, but space isn't allocated yet. This might cause a SIGBUS once we write into the mmapped space and the disk is full. Using fallocate might fail on some file systems. posix_fallocate is required to extend the file and allocate enough space even if the underlying filesystem would - normally return EOPNOTSUPP. */ + normally return EOPNOTSUPP. Note that we do also need to ftruncate + in case the maximum_size isn't known and the file needs to be shorter + because posix_fallocate can only extend. */ if (elf->parent == NULL && (elf->maximum_size == ~((size_t) 0) || (size_t) size > elf->maximum_size) + && unlikely (ftruncate (elf->fildes, size) != 0) && unlikely (posix_fallocate (elf->fildes, 0, size) != 0)) { __libelf_seterrno (ELF_E_WRITE_ERROR); diff --git a/tests/ChangeLog b/tests/ChangeLog index 19878ac..34f89cc 100644 --- a/tests/ChangeLog +++ b/tests/ChangeLog @@ -1,3 +1,8 @@ +2015-06-16 Mark Wielaard <[email protected]> + + * run-strip-test.sh: Add strip-in-place (eu-strip without -o) test + for non-ET_REL files. + 2015-05-30 Mark Wielaard <[email protected]> * backtrace-subr.sh (check_native_core): Notice core file couldn't be diff --git a/tests/run-strip-test.sh b/tests/run-strip-test.sh index c558e90..2ebb5a9 100755 --- a/tests/run-strip-test.sh +++ b/tests/run-strip-test.sh @@ -49,6 +49,19 @@ testrun ${abs_top_builddir}/src/unstrip -o testfile.unstrip testfile.temp testfi testrun ${abs_top_builddir}/src/elfcmp --hash-inexact $original testfile.unstrip } +# Now strip in-place and make sure it is smaller. +# Skip ET_REL files, they might have unexpected symbol table entries. +is_ET_REL=0 +testrun ${abs_top_builddir}/src/readelf -h $original 2>&1 \ + | fgrep 'REL (Relocatable file)' && is_ET_REL=1 +if test $is_ET_REL -eq 0; then + SIZE_original=$(stat -c%s $original) + testrun ${abs_top_builddir}/src/strip $original + SIZE_stripped=$(stat -c%s $original) + test $SIZE_stripped -lt $SIZE_original || + { echo "*** failure in-place strip file not smaller $original"; status=1; } +fi + tempfiles testfile.sections testrun ${abs_top_builddir}/src/readelf -S testfile.temp > testfile.sections || status=$? fgrep ' .debug_' testfile.sections && status=1 -- 1.8.3.1
