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

[Don Zickus]

On Sat, Dec 10, 2016 at 01:41:03PM +0100, Greg Kroah-Hartman wrote:
> On Fri, Dec 09, 2016 at 11:46:54PM +0100, Dodji Seketeli wrote:
> > Hello,
> > 
> > Nicholas Piggin  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)
> > 
> > 
> > 
> > real0m2.595s
> > user0m2.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
> > 
> > 
> > real0m2.574s
> > user0m2.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.)

Well, genksyms does provide this today with the .symref files.  It may not
be as thorough and flexible as libabigail, but RH has been using it for
years to quickly determine what patches broke the abi and more importantly
where (which can be challenging).  I just didn't want to downplay what is
available today.

On the flip side, I do like what libabigail has to offer.  There seems to be
some interesting new ways of handling our abi and I look forward to our kabi
team putting it to use. :-)

> 
> 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 :)

I also don't want folks to forget that are two parts to this equation.  The
checking above is the first part.  But the second part is what to do about
the stuff you ignored, which leads to the run time checks.

If you don't maintain 100% abi (which RH doesn't), then we need a way to
block drivers from loading that use symbols which we do not maintain and
broke.  The crc checks at load time work great for this.  Hopefully we can
continue to support what modversions is providing today (or something
similar).

I do not think vermagic will be usable at all for us.

Thanks!

Cheers,
Don

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

[Don Zickus]

On Fri, Dec 09, 2016 at 01:50:41PM +1000, Nicholas Piggin wrote:
> > 
> > We have plenty of customers with 10 year old drivers, where the expertise
> > has long left the company.  The engineers still around, recompile and make
> > tweaks to get things working on the latest RHEL.  Verify it passes testing
> > and release it.  Then they hope to not touch it again for a few years until
> > the next RHEL comes along.
> > 
> > Scary, huh? :-)
> 
> Oh yeah my aim here is not to make distro or out of tree module vendors
> life harder, actually the opposite. If it turns out modversions really is
> the best approach, I'm not in a position to complain about its complexity
> because we have Suse and Redhat people maintaining the build and module
> systems :) I just want to see if we can do things better.

Hi Nick,

I think we are in pretty good agreement here.  We can do better than
modversions.  On the flip side, I would hate to see modversions ripped out
until we have an alternate path forward as it does get us by for now. :-)

Cheers,
Don

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

[Don Zickus]

On Thu, Dec 01, 2016 at 05:06:11PM +0100, Greg Kroah-Hartman wrote:
> On Thu, Dec 01, 2016 at 10:40:59AM -0500, Don Zickus wrote:
> > Unfortunately, there are various drivers that will never go upstream
> > 
> > - paid storage drivers that provide bells and whistles on top of inbox
> >   driver
> 
> That's because the developer doesn't want them upstream, that's their
> fault, nothing we can do about them.
> 
> > - old drivers/fs that application has been relying on for a long time but
> >   company doesn't have resources to migrate to current technology.
> 
> That's what drivers/staging/ is for, I'll take anything that builds (and
> sometimes stuff that doesn't build) as long as people are actually using
> it.  So send the stuff that is in this category on to me and that will
> reduce your burden a _lot_.

Hi Greg,

I will forward this offer to the right folks and see who we can get to bite.
:-)  Thanks!

Cheers,
Don

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

[Don Zickus]

On Thu, Dec 01, 2016 at 07:26:09AM -0800, Christoph Hellwig wrote:
> On Thu, Dec 01, 2016 at 10:20:39AM -0500, Don Zickus wrote:
> > 
> > - provide the memory allocation (instead of having the driver staticly
> >   allocate)
> > - provide functions to retrieve various internal data (instead of having the
> >   driver do direct referencing to deep internal elements)
> > - cut down on some static inlines (and use accessory functions instead),
> >   etc.
> > 
> > Those types of changes allow the OOT driver to be more ignorant of kernel
> > changes and struct modifications.
> 
> All that is counter to what we really want to have:  a well integrated
> kernel that moves forward together so that we can see and improve the
> whole situation.  No need to make things worse just to help leeches.
> Get your damn drivers upstream ASAP and let's stop this discussion..

I understand and won't disagree with you. :-)

Unfortunately, there are various drivers that will never go upstream

- paid storage drivers that provide bells and whistles on top of inbox
  driver
- old drivers/fs that application has been relying on for a long time but
  company doesn't have resources to migrate to current technology.

We have been trying over the years to do what we can to move customers in
the right direction.  It is just a slow process, sadly.

Cheers,
Don

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

[Don Zickus]

On Thu, Dec 01, 2016 at 03:32:15PM +1100, Nicholas Piggin wrote:
> > Anyway, MODVERSIONS is our way of protecting our kabi for the last 10 years.
> > It isn't perfect and we have fixed the genksyms tool over the years, but so
> > far it mostly works fine.
> 
> Okay. It would be good to get all the distros in on this.
> 
> What I want to do is work out exactly what it is that modversions is
> giving you.
> 
> We know it's fairly nasty code to maintain and it does not detect ABI
> changes very well. But it's not such a burden that we can't maintain
> it if there are good reasons to keep it.

Hi Nick,

I won't disagree with you there. :-)

modversions is a pretty heavy handed approach that basically says if all the
symbols and types haven't changed for a given EXPORT_SYMBOL (recursively
checked), then there is a high degree of confidence the OOT driver will not
only load, but run correctly.

The question is how to provide a similar guarantee if a different way?

We have plenty of customers with 10 year old drivers, where the expertise
has long left the company.  The engineers still around, recompile and make
tweaks to get things working on the latest RHEL.  Verify it passes testing
and release it.  Then they hope to not touch it again for a few years until
the next RHEL comes along.

Scary, huh? :-)

Common examples, filesystems and storage drivers.


There is no way that I see to provide a 100% guarantee, but if we do enough
checks, we should be able to have a high degree of confidence the driver
won't blow up.

On the flip side, easy things in the kernel to do is:

- provide the memory allocation (instead of having the driver staticly
  allocate)
- provide functions to retrieve various internal data (instead of having the
  driver do direct referencing to deep internal elements)
- cut down on some static inlines (and use accessory functions instead),
  etc.

Those types of changes allow the OOT driver to be more ignorant of kernel
changes and struct modifications.


Look to Stanislav's responses for his ideas on new tooling.

Thanks for helping!

Cheers,
Don


> 
> > I am not sure what 'control vermagic' is, but it sounds like a string check,
> > which won't protect against the boatload of backports we do to structs,
> > enums, and functions.
> 
> Basically vermagic is the string all modules and the kernel get, which
> must match in order to load modules. If you have modversions disabled,
> then vermagic includes the kernel version. If modversions is enabled,
> then vermagic does not include the kernel version but the CRCs have to
> also match.
> 
> Controlling it explicitly is just a couple of lines where a distro can
> control it (so they can update their kernel version without breaking).
> It's not meant to solve everything, just the first one.
>  
> > Currently we are exploring various ways to get smarter here.  The genksyms
> > tool has its limitations and handling kabi hacks in RHEL is getting
> > tiresome.
> > 
> > I think GregKH pointed to one such tool, libabigail?  We are working on
> > others too.
> > 
> > 
> > Circling back to enabling MODVERSIONS in Fedora, that was to start the
> > process of syncing Fedora with RHEL stuff in preparation for smarter tools.
> > 
> > 
> > If you take away MODVERSIONS, that would put a damper in our work, but
> > easily carried privately (much like MODSIGNING for 8 years until it went
> > upstream :-) ).
> 
> I don't think that's necessary. A feature requirement for a distro is just
> as valid as any other user of upstream. I don't want to hinder any distro,
> I'm just still not quite seeing the big picture of exactly what functionality
> you need from the kernel.
> 
> Thanks,
> Nick

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

[Don Zickus]

On Wed, Nov 30, 2016 at 10:40:02AM -0800, Linus Torvalds wrote:
> On Wed, Nov 30, 2016 at 10:18 AM, Nicholas Piggin  wrote:
> >
> > Here's an initial rough hack at removing modversions. It gives an idea
> > of the complexity we're carrying for this feature (keeping in mind most
> > of the lines removed are generated parser).
> 
> You definitely don't have to try to convince me. We've had many issues
> with modversions over the years. This was just the "last drop" as far
> as I'm concerned, we've had random odd crc generation failures due to
> some build races too.
> 
> > In its place I just added a simple config option to override vermagic
> > so distros can manage it entirely themselves.
> 
> So at least Fedora doesn't even enable CONFIG_MODVERSIONS as-is. I'm
> _hoping_ it's just Debian that wants this, and we'd need to get some
> input from the Debian people whether that "control vermagic" is
> sufficient? I suspect it isn't, but I can't come up with any simple
> alternate model either..

Oddly, I just posted a patch to enable this for Fedora and then someone
pointed me at this thread. :-/

Sorry for chiming in late, but yes RHEL is a big user of MODVERSIONS for our
kabi protection work.  Despite our best intentions we still have lots of
partners and customers that provide value-add out-of-tree drivers to their
customers.  These module builders requested we have a mechanism to allow
rolling modules forward for each of our minor RHEL updates without breaking
their drivers.

They requested this to save time and money on rebuilding and retesting.  It
also helps deal with situations where RHEL puts out a security fix or new
minor release and the provider of OOT driver has not released the
appropriate update.  Customers like the ability to roll their special
drivers forward quickly to their schedule.

Now we don't protect every symbol, just a select few that our meets our
customers needs (and developers willing to support it).

Anyway, MODVERSIONS is our way of protecting our kabi for the last 10 years.
It isn't perfect and we have fixed the genksyms tool over the years, but so
far it mostly works fine.

I am not sure what 'control vermagic' is, but it sounds like a string check,
which won't protect against the boatload of backports we do to structs,
enums, and functions.

Currently we are exploring various ways to get smarter here.  The genksyms
tool has its limitations and handling kabi hacks in RHEL is getting
tiresome.

I think GregKH pointed to one such tool, libabigail?  We are working on
others too.


Circling back to enabling MODVERSIONS in Fedora, that was to start the
process of syncing Fedora with RHEL stuff in preparation for smarter tools.


If you take away MODVERSIONS, that would put a damper in our work, but
easily carried privately (much like MODSIGNING for 8 years until it went
upstream :-) ).


We would prefer to work with various folks to figure out a better solution
to solve our/others needs.  Anyone interested in working with Red Hat should
contact Stanislav Kozina (skoz...@redhat.com) (cc'd above) and cc myself.

Cheers,
Don



> 
> I'm also somewhat surprised that it's Debian that has this problem,
> considering how Debian is usually the distro that is _least_ receptive
> to various non-free binaries.
> 
> Linus

Bug#608138: [PATCH v2] watchdog: Improve initialisation error message and documentation

[Don Zickus]

On Sun, Jan 02, 2011 at 11:02:42PM +, Ben Hutchings wrote:
 The error message 'NMI watchdog failed to create perf event...'  does
 not make it clear that this is a fatal error for the watchdog.  It
 also currently prints the error value as a pointer, rather than
 extracting the error code with PTR_ERR().  Fix that.
 
 Add a note to the description of the 'nowatchdog' kernel parameter to
 associate it with this message.
 
 Reported-by: Cesare Leonardi celeo...@gmail.com
 Signed-off-by: Ben Hutchings b...@decadent.org.uk

Looks good, thanks.  I'll add it to my queue.

Cheers,
Don



-- 
To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/20110103135602.go3...@redhat.com