I fought enough rounds with this libelf problem to get my head muddled
about it, and then it's been several days since I thought hard about it.
So now I just want to air everything I think I know about it before it rots
any further in my brain. Some fresh eyeballs on this and checking up my
logic would help a lot.
In <= 0.137, elf_update could crash if the application set ELF_F_LAYOUT and
then made some sh_offset+sh_size ranges that overlap. This happened in
eu-strip with a botched input file.
https://bugzilla.redhat.com/show_bug.cgi?id=476136 is for that crash (and
some others). The case was: eu-strip -o foo try.out with input file
https://bugzilla.redhat.com/attachment.cgi?id=322583
In commit 75b07c00/commit 534ad315, I fixed that case.
This code shipped in 0.138, and is still current on the trunk.
This caused a regression in a different case. It broke rpmbuild's
debugedit on some files (killing rawhide kernel builds), which started
hitting a new assert. debugedit uses ELF_F_LAYOUT, and then dirties only
one or two of several sections. Here the case was some .ko file (ET_REL).
I think the situation was that a dirty section with an odd size
(e.g. .debug_str, with sh_addralign=1) is followed by a non-dirty section
that requires some filling (sh_addralign>1), or some similar interleaving
of sections. It crashed in the filling code after the bookeeping got
confused by skipping the non-dirty sections. I'm having trouble tracking
down the reproducer for that one right now. (It was in rawhide koji failed
builds that have been GC'd.) I'll keep looking.
On roland/pending I have commit ed9d3bc1. This fixed the crash. (This is
in rawhide only, 0.138-2.) However, it means that elf_update will touch
all the inter-section alignment fill bytes every time, even ones between
two non-dirty sections that didn't move at all. That seems suboptimal.
In retrospect it occurred to me that 0.137 might have been wrong in certain
cases. Say you have an odd-sized section followed by an aligned section,
so there is fill space between them. Say the odd-sized section had been 3,
and the next section is aligned to 4. Now you change that section's
sh_size to 1, and mark it dirty. elf_update won't write those fill bytes.
What should the general rule be here? It's not clear e.g. that one should
not elf_begin(,ELF_C_RDWR,) + elf_fill(,0xaa) + elf_update(,) to change
all the inter-section fill bytes.
Off hand what seems like the rule that is simplest to grok, while not being
ridiculously over-eager to write fill bytes, is when you dirty a section,
then any fill bytes before that section required by its alignment get
written. Otherwise, they don't. (Here the section headers count as "a
section" that might have been dirtied.)
That might even have been what the rule was before. But I've fiddled with
it too much and become unclear now.
We should have test cases for these three scenarios in our suite.
Note that affected code path has mmap and non-mmap variants that
are similar but don't share code. So proper regression tests will
try both ELF_C_RDWR and ELF_C_RDWR_MMAP.
https://bugzilla.redhat.com/attachment.cgi?id=322583 (try.out)
is large and of unknown provenance, so we can't just use it.
We need to construct a test file that tickles the same problem.
I'm trying to hit the debugedit regression now with:
pkgs/kernel/devel
cvs up -r kernel-2_6_29-0_9_rc0_git4_fc11
make x86_64
on F-10 with elfutils-0.138-1.fc10.x86_64 (updates-testing, buggy,
should behave the same as current trunk code, i.e. w/o commit ed9d3bc1).
This is after I failed to find a file more easily that triggered using the
test case below. This might fail to correctly mimic the debugedit behavior
that matters. Or there might be some necessary weirdness about the
particular test file required that I haven't understood correctly.
(I tried 'mkdir k; eu-unstrip -am -K -d k' on F-10 and tried all those
.ko files with no hits of the bug.)
The third case is purely theoretical. If it really existed, we should
construct a test case to tickle it and verify that it makes <= 0.137 misbehave.
Thanks,
Roland
=====
/* Copyright (C) 2009 Red Hat, Inc.
This file is part of Red Hat elfutils.
Red Hat elfutils 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; version 2 of the License.
Red Hat 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 Red Hat elfutils; if not, write to the Free Software Foundation,
Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301 USA.
Red Hat elfutils is an included package of the Open Invention Network.
An included package of the Open Invention Network is a package for which
Open Invention Network licensees cross-license their patents. No patent
license is granted, either expressly or impliedly, by designation as an
included package. Should you wish to participate in the Open Invention
Network licensing program, please visit
www.openinventionnetwork.com
<
http://www.openinventionnetwork.com>. */
#include <errno.h>
#include <error.h>
#include <fcntl.h>
#include <gelf.h>
#include <stdlib.h>
#include <unistd.h>
#include <string.h>
static void
handle_file (const char *file, Elf_Cmd cmd)
{
int fd = open (file, O_RDWR);
if (fd < 0)
error (EXIT_FAILURE, errno, "cannot open input file '%s'", file);
Elf *elf = elf_begin (fd, cmd, NULL);
size_t shstrndx;
if (elf_getshstrndx (elf, &shstrndx))
error (EXIT_FAILURE, 0, "problems opening '%s' as ELF file: %s",
file, elf_errmsg (-1));
Elf_Scn *scn = NULL;
while ((scn = elf_nextscn (elf, scn)) != NULL)
{
GElf_Shdr shdr_mem;
const char *name = elf_strptr (elf, shstrndx,
gelf_getshdr (scn, &shdr_mem)->sh_name);
if (name != NULL && !strncmp (name, ".debug", 6))
elf_flagscn (scn, ELF_C_SET, ELF_F_DIRTY);
}
elf_flagelf (elf, ELF_C_SET, ELF_F_LAYOUT);
if (elf_update (elf, ELF_C_WRITE) == -1)
error (EXIT_FAILURE, 0, "elf_update failed: %s", elf_errmsg (-1));
elf_end (elf);
close (fd);
}
int
main (int argc, char *argv[])
{
elf_version (EV_CURRENT);
for (int i = 1; i < argc; ++i)
{
handle_file (argv[i], ELF_C_RDWR_MMAP);
handle_file (argv[i], ELF_C_RDWR);
}
return 0;
}