This is the mail archive of the
mailing list for the elfutils project.
Re: libelf RDWR and elf_newscn do not work
- From: Jiri Slaby <jslaby at suse dot cz>
- To: elfutils-devel at lists dot fedorahosted dot org
- Date: Tue, 05 Nov 2013 20:38:02 +0100
- Subject: Re: libelf RDWR and elf_newscn do not work
On 11/05/2013 04:31 PM, Mark Wielaard wrote:
> On Fri, 2013-10-11 at 13:36 +0200, Mark Wielaard wrote:
>> On Thu, 2013-10-10 at 17:34 +0200, Jiri Slaby wrote:
>>> On 10/10/2013 03:16 PM, Mark Wielaard wrote:
>>>> @@ -764,7 +765,8 @@ __elfw2(LIBELFBITS,updatefile) (Elf *elf, int change_bo, size_t shnum)
>>>> (*shdr_fctp) (&shdr_data[scn->index],
>>>> sizeof (ElfW2(LIBELFBITS,Shdr)), 1);
>>>> - else if (elf->state.ELFW(elf,LIBELFBITS).shdr == NULL)
>>>> + else if (elf->state.ELFW(elf,LIBELFBITS).shdr == NULL
>>>> + || (elf->flags & ELF_F_DIRTY))
>>> I seem to miss where is elf->flags |= ELF_F_DIRTY in the newscn path...
>>> Should it be added too?
>> No, I don't think it should be set there. But you do raise a good point.
>> I had assumed that since newscn increases e_shnum it would mark the
>> whole Elf dirty. But now that I look it doesn't seem to. if e_shnum is
>> changed then the ehdr->flags do get ELF_F_DIRTY set (see elf_update ->
>> elf32_updatenull). But I cannot immediately see why the whole Elf file
>> is marked dirty (although it is in your example).
>> So either I am missing something that makes it correct anyway, or the
>> check should be against ehdr->flags. I am digging...
> So elf_update will first determine the new number of sections from the
> list updated by newscn. Then it will call updatenull to determine what
> actually changed, including comparing the new shnum with the
> ehdr->e_shnum with update_if_changed, which will set the ehdr_flags to
> ELF_F_DIRTY. Then ehdr is written and the ehdr_flags dirty bit is
> cleared. So we cannot directly use that.
> But update_null will also calculate the new ehdr->e_shoff, which will
> change is any section is added/removed/changed size, etc. And in that
> case it will set the ELF_F_DIRTY bit of the elf->flags. So I do think
> the patch is correct.
Ok, it makes sense, do you plan to push the patch?