linux-kernel - Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Mon, 12 Dec 2016 13:50:06 +1000
From:   Nicholas Piggin <npiggin@...il.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     Dodji Seketeli <dodji@...eteli.org>,
        Ian Campbell <ijc@...lion.org.uk>,
        Michal Marek <mmarek@...e.com>,
        Ben Hutchings <ben@...adent.org.uk>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Adam Borowski <kilobyte@...band.pl>,
        Linux Kbuild mailing list <linux-kbuild@...r.kernel.org>,
        Debian kernel maintainers <debian-kernel@...ts.debian.org>,
        "linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>,
        Arnd Bergmann <arnd@...db.de>, Ingo Molnar <mingo@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Stanislav Kozina <skozina@...hat.com>
Subject: Re: [PATCH] x86/kbuild: enable modversions for symbols exported
 from asm

On Sat, 10 Dec 2016 13:41:03 +0100
Greg Kroah-Hartman <gregkh@...uxfoundation.org> wrote:

> On Fri, Dec 09, 2016 at 11:46:54PM +0100, Dodji Seketeli wrote:
> > Hello,
> > 
> > Nicholas Piggin <npiggin@...il.com> a écrit:
> > 
> > [...]
> >   
> > > That said, a dwarf based checker tool should be able to do as good a job
> > > (maybe a bit better because report is very informative and it may pick up
> > > compiler alignments or padding options).  
> > 
> > So, Nicholas was kind enough to send me the two Linux Kernel binaries
> > that he built with the tiny little interface change that we were
> > discussing earlier.  Here is what the abidiff[1] tools says about that
> > interface change:
> > 
> >     $ time ~/git/libabigail/kabidiff/build/tools/abidiff vmlinux.abi1.abi vmlinux.abi2.abi
> >     Functions changes summary: 0 Removed, 1 Changed, 0 Added function
> >     Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
> > 
> >     1 function with some indirect sub-type change:
> > 
> >       [C]'function int foo(blah*)' at memory.c:82:1 has some indirect sub-type changes:
> >         parameter 1 of type 'blah*' has sub-type changes:
> >           in pointed to type 'struct blah' at memory.c:78:1:
> >             type size changed from 32 to 64 bits
> >             1 data member insertion:
> >               'int blah::y', at offset 0 (in bits) at memory.c:79:1
> >             1 data member change:
> >              'int blah::x' offset changed from 0 to 32 (in bits) (by +32 bits)
> > 
> > 
> > 
> >     real	0m2.595s
> >     user	0m2.489s
> >     sys	0m0.108s
> >     $ 
> > 
> > I kept the timing information to give you an idea of the time it takes
> > on a non-optimized build of abidiff.
> > 
> > One could for instance want that types that are not defined in header
> > files be kept out of the change report.  In that case it's possible to
> > write a little suppression specification file like this one:
> > 
> >     $ cat vmlinux.abignore 
> >     [suppress_type]
> >       source_location_not_regexp = .*\\.h
> >     $
> > 
> > You can then pass that suppression file to the tool:
> > 
> >     $ ~/git/libabigail/kabidiff/build/tools/abidiff --suppr vmlinux.abignore vmlinux.abi1.abi vmlinux.abi2.abi
> >     Functions changes summary: 0 Removed, 0 Changed (1 filtered out), 0 Added function
> >     Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
> > 
> > 
> >     real	0m2.574s
> >     user	0m2.473s
> >     sys	0m0.102s
> >     $
> > 
> > So this is the kind of interface change analysis tool we are working on
> > at the moment.
> > 
> > One could also imagine a tool that would compute a CRC that takes the
> > very same suppression specification files into account, letting people
> > to decide that some interface changes are OK.  That CRC would thus be
> > added to the special ELF sections we already have today.  We could keep
> > the modversion machinery, but with a greater dose of flexibility.
> > Whenever modversion detects a change, abidiff would tell people what the
> > change is exactly.
> > 
> > What do you guys think?  
> 
> YES YES YES!!!
> 
> Now I don't work on a distro anymore, but I would think that something
> like this would be really useful, pointing out exactly what changed is
> very important for distro maintainers to determine what they want to do
> (either fix up the abi change with strange hacks, or ignore it due to
> the change being in an area they don't care at all about, i.e. a random
> driver subsystem.)
> 
> So yes, I think this is really good stuff.  But if the distro
> maintainers correct me and think it's useless, then I need to revisit my
> view of exactly what they do for their customers :)

Agree completely. BTW (for those who might be looking into these tools),
we also have https://github.com/skozina/kabi-dw that Stanislav (cc'ed)
mentioned earlier.

It's true that the current modversions __crc_ matching infrastructure is
"just" a symbol versioning system, and we could keep it and just populate
it with something other than genksyms (e.g., a symbol version list provided
by distros). But the starting point should be *no* versioning and simply
using names to break linkage. Unless there's a compelling reason not to,
symbols are simpler, easier, everyone knows how they work.

The other question would be whether to pull a minimal tool into the kernel
source or keep them out of tree (but possibly add some helper scripts etc).
I guess we'll need to see what distros want.

Thanks,
Nick