Re: [PATCH v1] abg-dwarf-reader: detect kernel modules without exports as such

["Matthias Maennich via libabigail" <libabigail@sourceware.org>]

Hi Mark!

On Wed, Jul 24, 2019 at 11:57:10PM +0200, Mark Wielaard wrote:
> Hi,
> 
> On Wed, Jul 24, 2019 at 10:32:55PM +0100, Matthias Maennich via libabigail wrote:
> > Kernel modules without exported symbols (no use of EXPORT_SYMBOL*()),
> > will not have a __ksymtab_strings section. Libabigail will therefore
> > assume they are usual ELF binaries. That leads to wrong results as
> > now all ELF symbols are considered part of the ABI. That is obviously
> > wrong. Instead consider binaries having a .modinfo section to be kernel
> > binaries. We keep the __ksymtab_strings condition as vmlinux has no
> > .modinfo section but a __ksymtab_strings if symbols are exported.
> > [...] 
> > 	* src/abg-dwarf-reader.cc(is_linux_kernel_binary): consider
> > 	  binaries only having a .modinfo section to be kernel binaries
> > [...]
> >    bool
> >    is_linux_kernel_binary() const
> > -  {return find_section(elf_handle(), "__ksymtab_strings", SHT_PROGBITS);}
> > +  {
> > +    return find_section(elf_handle(), "__ksymtab_strings", SHT_PROGBITS)
> > +	   || find_section(elf_handle(), ".modinfo", SHT_PROGBITS);
> > +  }
> 
> I think the change itself is correct and better than what is there
> now.
> 
> But this is interesting to me because we are discussing an
> eu-elfclassify program addition for elfutils. That utility at the
> moment classifies a --linux-kernel-module as:
> 
> /* Returns true if the file is a linux kernel module (is ET_REL and
>    has the two magic sections .modinfo and .gnu.linkonce.this_module).  */
> static bool
> is_linux_kernel_module (void)
> {
>   return (elf_kind (elf) == ELF_K_ELF
>           && elf_type == ET_REL
>           && has_modinfo
>           && has_gnu_linkonce_this_module);
> }
> 
> Just wondering if you believe that check is too strict?
> 
> The reason to check for both "magic" section names was mainly because
> ".modinfo" seemed a little too generic.
> 
> One difference is that eu-elfclassify doesn't check the section type.
> Maybe it should do that like here, to make sure that it is
> SHT_PROGBITS.
> 
To be honest, I am not entirely sure myself. I was told in IRC that the check
above is sufficient and should be good for kernels of at least 3.18+ (possibly
older kernels as well). I will add a patch on top to restrict abigail a bit
more on this.

> Sorry for hijacking this patch review for something slightly
> different. But I was happy to see something very similar to what I was
> working on in a different, but somewhat related project, and was
> wondering whether it was the correct way to classify something as a
> kernel module.
> 
> Cheers,
> 
> Mark

-- 
Cheers,
Matthias