[Rpm-maint] [rpm-software-management/rpm] Support build-id generation from compressed ELF files (#653)

Mark Wielaard mark at klomp.org
Tue Apr 9 22:03:29 UTC 2019


On Tue, 2019-04-09 at 09:31 +0000, Florian Festi wrote:
> This should probably be reviewed by Mark Wielaard or some kernel
> person. This is all gibberish to me.

It isn't that bad is it?

TLDR; Looks good to me.

> +static int haveModinfo(Elf *elf)
> +{
> +    Elf_Scn * scn = NULL;
> +    size_t shstrndx;
> +    int have_modinfo = 0;
> +    const char *sname;
> +
> +    if (elf_getshdrstrndx(elf, &shstrndx) == 0) {
> +       while ((scn = elf_nextscn(elf, scn)) != NULL) {
> +           GElf_Shdr shdr_mem, *shdr = gelf_getshdr(scn, &shdr_mem);
> +           if (shdr == NULL)
> +               continue;
> +           sname = elf_strptr(elf, shstrndx, shdr->sh_name);
> +           if (sname && rstreq(sname, ".modinfo")) {
> +               have_modinfo = 1;
> +               break;
> +           }
> +       }
> +    }
> +    return have_modinfo;
> +}

Looks like a reasonable way to check the file has a .modinfo section.
You could add some more sanity checks, but it is unlikely that anything
else has a .modinfo section anyway. And you do another check below.

> @@ -1803,15 +1825,14 @@ static int generateBuildIDs(FileList fl, ARGV_t *files)
>             int fd = open (flp->diskPath, O_RDONLY);
>             if (fd >= 0) {
>                 /* Only real ELF files, that are ET_EXEC, ET_DYN or
> -                  kernel modules (ET_REL files with names ending in .ko)
> +                  kernel modules (ET_REL files with .modinfo section)
>                    should have build-ids. */
>                 GElf_Ehdr ehdr;
>                 Elf *elf = elf_begin (fd, ELF_C_READ, NULL);
>                 if (elf != NULL && elf_kind(elf) == ELF_K_ELF
>                     && gelf_getehdr(elf, &ehdr) != NULL
>                     && (ehdr.e_type == ET_EXEC || ehdr.e_type == ET_DYN
> -                       || (ehdr.e_type == ET_REL
> -                           && rpmFileHasSuffix (flp->diskPath, ".ko")))) {
> +                       || (ehdr.e_type == ET_REL && haveModinfo(elf)))) {

Yes, definitely a better check that the "file ends with .ko" one.

> +#if HAVE_DWELF_ELF_BEGIN
> +               Elf *elf = dwelf_elf_begin(fd);
> +#else
>                 Elf *elf = elf_begin (fd, ELF_C_READ, NULL);
> +#endif

That is indeed how dwelf_elf_begin was intended to be used. As a drop
in replacement for elf_begin (fd, ELF_C_READ, NULL) that "magically"
handles compressed ELF files.

> +      # whether libdw supports compressed ELF objects
> +      AC_CHECK_LIB(dw, dwelf_elf_begin, [
> +                   AC_DEFINE(HAVE_DWELF_ELF_BEGIN, 1, [Have dwelf_elf_begin?])

Looks like the correct check.



More information about the Rpm-maint mailing list