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], >>>> scn->shdr.ELFW(e,LIBELFBITS), >>>> 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? -- js suse labs
