Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1528743 > unrolled thread

Re: BUG: 4.9-rc6 Still "no symbol version" on boot

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: BUG: 4.9-rc6 Still "no symbol version" on boot Philip Müller <philm@manjaro.org> - 2016-11-23 21:10 +0100
    Re: BUG: 4.9-rc6 Still "no symbol version" on boot Robert LeBlanc <robert@leblancnet.us> - 2016-11-23 21:20 +0100
      Re: BUG: 4.9-rc6 Still "no symbol version" on boot Philip Müller <philm@manjaro.org> - 2016-11-23 21:30 +0100
    Re: BUG: 4.9-rc6 Still "no symbol version" on boot Adam Borowski <kilobyte@angband.pl> - 2016-11-23 22:00 +0100
      Re: BUG: 4.9-rc6 Still "no symbol version" on boot Robert LeBlanc <robert@leblancnet.us> - 2016-11-23 22:10 +0100
      [PATCH] x86/kbuild: enable modversions for symbols exported from asm Adam Borowski <kilobyte@angband.pl> - 2016-11-23 23:10 +0100
        Re: [PATCH] x86/kbuild: enable modversions for symbols exported from  asm Philip Müller <philm@manjaro.org> - 2016-11-24 00:20 +0100
        Re: [PATCH] x86/kbuild: enable modversions for symbols exported from  asm Ingo Molnar <mingo@kernel.org> - 2016-11-24 06:10 +0100
          Re: [PATCH] x86/kbuild: enable modversions for symbols exported  from asm Nicholas Piggin <npiggin@gmail.com> - 2016-11-24 06:30 +0100
            Re: [PATCH] x86/kbuild: enable modversions for symbols exported from  asm Ingo Molnar <mingo@kernel.org> - 2016-11-24 07:10 +0100
              Re: [PATCH] x86/kbuild: enable modversions for symbols exported  from asm Nicholas Piggin <npiggin@gmail.com> - 2016-11-24 08:30 +0100
                Re: [PATCH] x86/kbuild: enable modversions for symbols exported from  asm Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-11-24 08:40 +0100
                  Re: [PATCH] x86/kbuild: enable modversions for symbols exported  from asm Nicholas Piggin <npiggin@gmail.com> - 2016-11-24 09:00 +0100
                    Re: [PATCH] x86/kbuild: enable modversions for symbols exported from  asm Michal Marek <mmarek@suse.com> - 2016-11-24 10:40 +0100
                      Re: [PATCH] x86/kbuild: enable modversions for symbols exported  from asm Nicholas Piggin <npiggin@gmail.com> - 2016-11-24 11:10 +0100
                        Re: [PATCH] x86/kbuild: enable modversions for symbols exported from  asm Michal Marek <mmarek@suse.com> - 2016-11-24 12:00 +0100
                    Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Arnd Bergmann <arnd@arndb.de> - 2016-11-24 10:50 +0100
                      Re: [PATCH] x86/kbuild: enable modversions for symbols exported  from asm Nicholas Piggin <npiggin@gmail.com> - 2016-11-24 11:10 +0100
                    Re: [PATCH] x86/kbuild: enable modversions for symbols exported from  asm Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-11-24 11:00 +0100
                      Re: [PATCH] x86/kbuild: enable modversions for symbols exported  from asm Nicholas Piggin <npiggin@gmail.com> - 2016-11-24 11:40 +0100
                        Re: [PATCH] x86/kbuild: enable modversions for symbols exported from  asm Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-11-24 16:30 +0100
                          Re: [PATCH] x86/kbuild: enable modversions for symbols exported  from asm Nicholas Piggin <npiggin@gmail.com> - 2016-11-25 01:50 +0100
                            Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Linus Torvalds <torvalds@linux-foundation.org> - 2016-11-25 19:10 +0100
                              Re: [PATCH] x86/kbuild: enable modversions for symbols exported  from asm Nicholas Piggin <npiggin@gmail.com> - 2016-11-26 02:00 +0100
                              Re: [PATCH] x86/kbuild: enable modversions for symbols exported  from asm Ben Hutchings <ben@decadent.org.uk> - 2016-11-29 02:20 +0100
                                Re: [PATCH] x86/kbuild: enable modversions for symbols exported  from asm Nicholas Piggin <npiggin@gmail.com> - 2016-11-29 03:40 +0100
                                  Re: [PATCH] x86/kbuild: enable modversions for symbols exported from  asm Michal Marek <mmarek@suse.com> - 2016-11-29 10:20 +0100
                                Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Linus Torvalds <torvalds@linux-foundation.org> - 2016-11-29 05:10 +0100
                                  Re: [PATCH] x86/kbuild: enable modversions for symbols exported from  asm Adam Borowski <kilobyte@angband.pl> - 2016-11-29 14:20 +0100
                                    Re: [PATCH] x86/kbuild: enable modversions for symbols exported from  asm Ingo Molnar <mingo@kernel.org> - 2016-11-29 14:40 +0100
                                      Re: [PATCH] x86/kbuild: enable modversions for symbols exported from  asm Adam Borowski <kilobyte@angband.pl> - 2016-11-29 15:30 +0100
                                    [PATCH] x86/kbuild: enable modversions for symbols exported from asm Adam Borowski <kilobyte@angband.pl> - 2016-11-29 15:00 +0100
                                      Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Linus Torvalds <torvalds@linux-foundation.org> - 2016-11-29 16:50 +0100
                                        Re: [PATCH] x86/kbuild: enable modversions for symbols exported from  asm Michal Marek <mmarek@suse.com> - 2016-11-29 17:20 +0100
                                          Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Linus Torvalds <torvalds@linux-foundation.org> - 2016-11-29 17:40 +0100
                                            Re: [PATCH] x86/kbuild: enable modversions for symbols exported from  asm Ben Hutchings <ben@decadent.org.uk> - 2016-11-29 21:00 +0100
                                              Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Linus Torvalds <torvalds@linux-foundation.org> - 2016-11-29 22:00 +0100
                                                Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Linus Torvalds <torvalds@linux-foundation.org> - 2016-11-30 20:00 +0100
                                                  Re: [PATCH] x86/kbuild: enable modversions for symbols exported  from asm Ben Hutchings <ben@decadent.org.uk> - 2016-11-30 22:40 +0100
                                                    Re: [PATCH] x86/kbuild: enable modversions for symbols exported  from asm Nicholas Piggin <npiggin@gmail.com> - 2016-12-01 03:20 +0100
                                                      Re: [PATCH] x86/kbuild: enable modversions for symbols exported  from asm Ben Hutchings <ben@decadent.org.uk> - 2016-12-01 03:40 +0100
                                                        Re: [PATCH] x86/kbuild: enable modversions for symbols exported  from asm Nicholas Piggin <npiggin@gmail.com> - 2016-12-01 05:00 +0100
                                                          Re: [PATCH] x86/kbuild: enable modversions for symbols exported from  asm Michal Marek <mmarek@suse.com> - 2016-12-01 17:40 +0100
                                                            Re: [PATCH] x86/kbuild: enable modversions for symbols exported from  asm Hannes Frederic Sowa <hannes@stressinduktion.org> - 2016-12-02 16:00 +0100
                                                            Re: [PATCH] x86/kbuild: enable modversions for symbols exported  from asm Nicholas Piggin <npiggin@gmail.com> - 2016-12-09 05:00 +0100
                                                              Re: [PATCH] x86/kbuild: enable modversions for symbols exported  from asm Ian Campbell <ijc@hellion.org.uk> - 2016-12-09 16:30 +0100
                                                                Re: [PATCH] x86/kbuild: enable modversions for symbols exported  from asm Nicholas Piggin <npiggin@gmail.com> - 2016-12-09 17:40 +0100
                                                                  Re: [PATCH] x86/kbuild: enable modversions for symbols exported from  asm Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-12-10 14:00 +0100
                                                  Re: [PATCH] x86/kbuild: enable modversions for symbols exported from  asm Don Zickus <dzickus@redhat.com> - 2016-12-01 05:50 +0100
                                                    Re: [PATCH] x86/kbuild: enable modversions for symbols exported  from asm Nicholas Piggin <npiggin@gmail.com> - 2016-12-01 05:50 +0100
                                                      Re: [PATCH] x86/kbuild: enable modversions for symbols exported from  asm Don Zickus <dzickus@redhat.com> - 2016-12-01 16:40 +0100
                                                        Re: [PATCH] x86/kbuild: enable modversions for symbols exported from  asm Don Zickus <dzickus@redhat.com> - 2016-12-01 17:00 +0100
                                                          Re: [PATCH] x86/kbuild: enable modversions for symbols exported from  asm Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-12-01 17:40 +0100
                                                            Re: [PATCH] x86/kbuild: enable modversions for symbols exported from  asm Don Zickus <dzickus@redhat.com> - 2016-12-01 20:10 +0100
                                                        Re: [PATCH] x86/kbuild: enable modversions for symbols exported from  asm Christoph Hellwig <hch@infradead.org> - 2016-12-01 17:10 +0100
                                                        Re: [PATCH] x86/kbuild: enable modversions for symbols exported  from asm Nicholas Piggin <npiggin@gmail.com> - 2016-12-09 05:10 +0100
                                                          Re: [PATCH] x86/kbuild: enable modversions for symbols exported from  asm Stanislav Kozina <skozina@redhat.com> - 2016-12-09 09:20 +0100
                                                            Re: [PATCH] x86/kbuild: enable modversions for symbols exported  from asm Nicholas Piggin <npiggin@gmail.com> - 2016-12-09 09:40 +0100
                                                              Re: [PATCH] x86/kbuild: enable modversions for symbols exported from  asm Stanislav Kozina <skozina@redhat.com> - 2016-12-09 16:00 +0100
                                                                Re: [PATCH] x86/kbuild: enable modversions for symbols exported  from asm Nicholas Piggin <npiggin@gmail.com> - 2016-12-09 17:20 +0100
                                                                  Re: [PATCH] x86/kbuild: enable modversions for symbols exported from  asm Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-12-09 17:30 +0100
                                                          Re: [PATCH] x86/kbuild: enable modversions for symbols exported from  asm Don Zickus <dzickus@redhat.com> - 2016-12-09 17:40 +0100
                                                    Re: [PATCH] x86/kbuild: enable modversions for symbols exported from  asm Stanislav Kozina <skozina@redhat.com> - 2016-12-01 12:20 +0100
                                                      Re: [PATCH] x86/kbuild: enable modversions for symbols exported  from asm Nicholas Piggin <npiggin@gmail.com> - 2016-12-01 12:30 +0100
                                                        Re: [PATCH] x86/kbuild: enable modversions for symbols exported from  asm Stanislav Kozina <skozina@redhat.com> - 2016-12-01 13:00 +0100
                                                          Re: [PATCH] x86/kbuild: enable modversions for symbols exported  from asm Nicholas Piggin <npiggin@gmail.com> - 2016-12-01 14:00 +0100
                                                        Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Dodji Seketeli <dodji@seketeli.org> - 2016-12-01 16:50 +0100
                                                    Re: [PATCH] x86/kbuild: enable modversions for symbols exported from  asm Michal Marek <mmarek@suse.com> - 2016-12-01 17:40 +0100
                                        Re: [PATCH] x86/kbuild: enable modversions for symbols exported from  asm Adam Borowski <kilobyte@angband.pl> - 2016-11-29 18:10 +0100
                                          Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Linus Torvalds <torvalds@linux-foundation.org> - 2016-11-29 18:30 +0100
                                            Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Linus Torvalds <torvalds@linux-foundation.org> - 2016-11-29 18:40 +0100
                                              Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Arnd Bergmann <arnd@arndb.de> - 2016-12-01 15:20 +0100
                                                Re: [PATCH] x86/kbuild: enable modversions for symbols exported from  asm Michal Marek <mmarek@suse.com> - 2016-12-01 17:40 +0100
                                                Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Linus Torvalds <torvalds@linux-foundation.org> - 2016-12-01 19:50 +0100
                                                  Re: [RFC, PATCH, v3.9] default exported asm symbols to zero Geert Uytterhoeven <geert@linux-m68k.org> - 2016-12-02 14:00 +0100
                                                    Re: [RFC, PATCH, v3.9] default exported asm symbols to zero Arnd Bergmann <arnd@arndb.de> - 2016-12-02 16:10 +0100
                                                  Re: [RFC, PATCH, v3.9] default exported asm symbols to zero Adam Borowski <kilobyte@angband.pl> - 2016-12-02 16:40 +0100
                                                  Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Linus Torvalds <torvalds@linux-foundation.org> - 2016-12-02 18:30 +0100
                                                  Re: [RFC, PATCH, v3.9] default exported asm symbols to zero Ben Hutchings <ben@decadent.org.uk> - 2016-12-03 05:40 +0100
                                                    Re: [RFC, PATCH, v3.9] default exported asm symbols to zero Arnd Bergmann <arnd@arndb.de> - 2016-12-03 12:10 +0100
                                                  Re: [PATCH] x86/kbuild: enable modversions for symbols exported from  asm Alan Modra <amodra@gmail.com> - 2016-12-04 09:20 +0100
                                                    Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Linus Torvalds <torvalds@linux-foundation.org> - 2016-12-04 22:10 +0100
                                            Re: [PATCH] x86/kbuild: enable modversions for symbols exported from  asm Michal Marek <mmarek@suse.com> - 2016-11-29 22:50 +0100
          Re: [PATCH] x86/kbuild: enable modversions for symbols exported from  asm Michal Marek <mmarek@suse.com> - 2016-11-24 10:30 +0100
        Regression: Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Kalle Valo <kvalo@codeaurora.org> - 2016-11-24 12:50 +0100
      Re: BUG: 4.9-rc6 Still "no symbol version" on boot Philip Müller <philm@manjaro.org> - 2016-11-24 00:10 +0100
        Re: BUG: 4.9-rc6 Still "no symbol version" on boot Robert LeBlanc <robert@leblancnet.us> - 2016-11-28 18:20 +0100

Page 2 of 5 — ← Prev page 1 [2] 3 4 5  Next page →

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

On Thu, Nov 24, 2016 at 09:31:52PM +1100, Nicholas Piggin wrote:
> On Thu, 24 Nov 2016 10:56:22 +0100
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> 
> > On Thu, Nov 24, 2016 at 06:53:22PM +1100, Nicholas Piggin wrote:
> > > On Thu, 24 Nov 2016 08:36:39 +0100
> > > Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> > >   
> > > > On Thu, Nov 24, 2016 at 06:20:26PM +1100, Nicholas Piggin wrote:  
> > > > > But still, modversions is pretty complicated for what it gives us. It sends
> > > > > preprocessed C into a C parser that makes CRCs using type definitions of
> > > > > exported symbols, then turns those CRCs into a linker script which which is
> > > > > used to link the .o file with. What we get in return is a quite limited and
> > > > > symbol "versioning" system.
> > > > > 
> > > > > What if we ripped all that out and just attached an explicit version to
> > > > > each export, and incompatible changes require an increment?    
> > > > 
> > > > How would that work for structures?  Would that be required for every
> > > > EXPORT_SYMBOL* somehow?  
> > > 
> > > Yeah just have EXPORT_SYMBOL take another parameter which attaches a version
> > > number and use that as the value for the __crc_ symbol versions rather than
> > > a calculated CRC.
> > > 
> > > Yes it would require some level of care from developers and may be a small
> > > annoyance when changing exports. But making people think a tiny bit more
> > > before chnaging exported ABI shouldn't be the end of the world.  
> > 
> > That wouldn't work at all for structures that change, as we never
> > explicitly "mark" them for export anywhere.
> 
> Well, the module arrives at the objects one way or another via an exported
> symbol. Although it can be by following a lot of pointers so yes it's
> probably near impossible to do well.

Yes, manual "marking" is never going to be a viable solution.

> >  You need a tool that looks
> > at either the source code (what we have today), or looks at the
> 
> What we have today only looks at the type of the exported function or
> variable I think (or does it? I didn't look that far into the parser).

It should catch things if you change a structure layout of something
that is an argument in a function (like a pointer to a structure),
otherwise it wouldn't really be that good of a check, and kind of
useless.

> Does not follow down all possible derivable pointer types.

It should be pretty good, as I think the code is based on the old SuSE
scripts that used to do this really well.  But it's been a long time
since I looked at it, so I could be wrong.

> > > > > Google tells me
> > > > > Linus is not a neutral bystander on the topic of symbol versioning, so I'm
> > > > > bracing for a robust response :) (actually I don't much care either way, I'm
> > > > > happy to put a couple of bandaids on it and keep it going)    
> > > > 
> > > > There are tools that people are working on to make it more obvious where
> > > > API breaks happen by looking at the .o debug data instead of our crazy
> > > > current system (which is really better than nothing), perhaps we should
> > > > start using them instead?
> > > > 
> > > > See here for more details about this:
> > > > 	https://kernel-recipes.org/en/2016/talks/would-an-abi-changes-visualization-tool-be-useful-to-linux-kernel-maintenance/  
> > > 
> > > Hmm. I guess it's basically similar to modversions, so has downsides of not
> > > detecting a semantic change unless it changes the type. But still, if we could
> > > replace our custom code with a tool like this for modversions functionality,
> > > that alone would be a massive improvement. But requiring debug info might be
> > > a bit of a show stopper. I also don't know if that would handle asm functions.  
> > 
> > I think we can live without asm functions changing their arguments as
> > that is usually very rare.  And maybe debugging info being a requirement
> > for those that want modversions (i.e. the distros), is ok as they
> > already generate that as part of their build.
> 
> Maybe. I'd like to know how people really care about it. Linus post from
> 
> http://yarchive.net/comp/linux/modversions.html
> 
> Seem to be that he just likes it to prevent module loading if the git version
> is not available. Fair usage, but could we do better with less effort? Maybe
> ship with a source version that can do the same job. If you take care of that
> case, then what is left?

The goal is to be able to tell when a symbol changed somehow (structure
or function signature) and if it has, then to hopefully prevent loading
a module that doesn't have the same signature.  The distros really want
this as they want "external" modules to load properly, even when they
bump their main kernel package version.  And it's a good goal to have,
no need to rebuild external packages (that usually come from external
places) if you don't have to, as sometimes you need those modules to
have your machine to work properly (like the fibre channel mess of
out-of-tree drivers...)

So however that type of checking is done, is fine with me, I have no
real desire to mess with this as personally, I never use it for my own
machines (I just use module signing and then throw away the key after
building the kernel).

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]

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

On Thu, 24 Nov 2016 16:24:10 +0100
Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:

> On Thu, Nov 24, 2016 at 09:31:52PM +1100, Nicholas Piggin wrote:
> > On Thu, 24 Nov 2016 10:56:22 +0100
> > Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> >   
> > > On Thu, Nov 24, 2016 at 06:53:22PM +1100, Nicholas Piggin wrote:  
> > > > On Thu, 24 Nov 2016 08:36:39 +0100
> > > > Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> > > >     
> > > > > On Thu, Nov 24, 2016 at 06:20:26PM +1100, Nicholas Piggin wrote:    
> > > > > > But still, modversions is pretty complicated for what it gives us. It sends
> > > > > > preprocessed C into a C parser that makes CRCs using type definitions of
> > > > > > exported symbols, then turns those CRCs into a linker script which which is
> > > > > > used to link the .o file with. What we get in return is a quite limited and
> > > > > > symbol "versioning" system.
> > > > > > 
> > > > > > What if we ripped all that out and just attached an explicit version to
> > > > > > each export, and incompatible changes require an increment?      
> > > > > 
> > > > > How would that work for structures?  Would that be required for every
> > > > > EXPORT_SYMBOL* somehow?    
> > > > 
> > > > Yeah just have EXPORT_SYMBOL take another parameter which attaches a version
> > > > number and use that as the value for the __crc_ symbol versions rather than
> > > > a calculated CRC.
> > > > 
> > > > Yes it would require some level of care from developers and may be a small
> > > > annoyance when changing exports. But making people think a tiny bit more
> > > > before chnaging exported ABI shouldn't be the end of the world.    
> > > 
> > > That wouldn't work at all for structures that change, as we never
> > > explicitly "mark" them for export anywhere.  
> > 
> > Well, the module arrives at the objects one way or another via an exported
> > symbol. Although it can be by following a lot of pointers so yes it's
> > probably near impossible to do well.  
> 
> Yes, manual "marking" is never going to be a viable solution.

I guess it really depends on how exactly you want to use it. For distros
that do stable ABI but rarely may have to break something for security
reasons, it should work and give exact control.

What else do people *actually* use it for? Preventing mismatched modules
when .git version is not attached and release version of the kernel has
not been bumped. Is that it?

> > >  You need a tool that looks
> > > at either the source code (what we have today), or looks at the  
> > 
> > What we have today only looks at the type of the exported function or
> > variable I think (or does it? I didn't look that far into the parser).  
> 
> It should catch things if you change a structure layout of something
> that is an argument in a function (like a pointer to a structure),
> otherwise it wouldn't really be that good of a check, and kind of
> useless.
>
> > Does not follow down all possible derivable pointer types.  
> 
> It should be pretty good, as I think the code is based on the old SuSE
> scripts that used to do this really well.  But it's been a long time
> since I looked at it, so I could be wrong.

Yeah... turns out modversions is in the "kind of useless" camp.

The crc is based on the name of the type and that's it (that's what I
thought, I just now verify it).

> > > > > > Google tells me
> > > > > > Linus is not a neutral bystander on the topic of symbol versioning, so I'm
> > > > > > bracing for a robust response :) (actually I don't much care either way, I'm
> > > > > > happy to put a couple of bandaids on it and keep it going)      
> > > > > 
> > > > > There are tools that people are working on to make it more obvious where
> > > > > API breaks happen by looking at the .o debug data instead of our crazy
> > > > > current system (which is really better than nothing), perhaps we should
> > > > > start using them instead?
> > > > > 
> > > > > See here for more details about this:
> > > > > 	https://kernel-recipes.org/en/2016/talks/would-an-abi-changes-visualization-tool-be-useful-to-linux-kernel-maintenance/    
> > > > 
> > > > Hmm. I guess it's basically similar to modversions, so has downsides of not
> > > > detecting a semantic change unless it changes the type. But still, if we could
> > > > replace our custom code with a tool like this for modversions functionality,
> > > > that alone would be a massive improvement. But requiring debug info might be
> > > > a bit of a show stopper. I also don't know if that would handle asm functions.    
> > > 
> > > I think we can live without asm functions changing their arguments as
> > > that is usually very rare.  And maybe debugging info being a requirement
> > > for those that want modversions (i.e. the distros), is ok as they
> > > already generate that as part of their build.  
> > 
> > Maybe. I'd like to know how people really care about it. Linus post from
> > 
> > http://yarchive.net/comp/linux/modversions.html
> > 
> > Seem to be that he just likes it to prevent module loading if the git version
> > is not available. Fair usage, but could we do better with less effort? Maybe
> > ship with a source version that can do the same job. If you take care of that
> > case, then what is left?  
> 
> The goal is to be able to tell when a symbol changed somehow (structure
> or function signature) and if it has, then to hopefully prevent loading
> a module that doesn't have the same signature.  The distros really want
> this as they want "external" modules to load properly, even when they
> bump their main kernel package version.  And it's a good goal to have,
> no need to rebuild external packages (that usually come from external
> places) if you don't have to, as sometimes you need those modules to
> have your machine to work properly (like the fibre channel mess of
> out-of-tree drivers...)
> 
> So however that type of checking is done, is fine with me, I have no
> real desire to mess with this as personally, I never use it for my own
> machines (I just use module signing and then throw away the key after
> building the kernel).

So we have:

1. The distro users. They don't break ABI, they really just want a way to
   avoid the kernel version check.

2. Linus or other kernel developer who wants to prevent a kernel
   accidentally loading out of date modules when they test some changes
   that don't bump kernel version.

3. Advanced end user who does not want to have to recompile their nvidia
   blob.

Anything else? So, how to handle them?

1. Distros may just want a way to avoid checking some minor part of the
   version string. In rare cases where they do break ABI, they can just
   rename the symbol: external modules have a distro/version compat layer
   anyway.

2. I wonder if this is still important 8 years later now that everyone
   uses git everywhere? :) I also don't think modversions helps this case
   reliably because we don't change export types all that often. Can we
   ship a git version in the source tree somehow that git can handle
   specially?

3. These people today are not well supported with modversions because it
   does not tell them about incompatibility of ABI. Semantics could change.
   Structure args could change. Objects derived through various pointers
   could change. We should not even bother trying IMO. Just let them do
   forced loading at their own risk. We shouldn't offer a false sense of
   security.

Thanks,
Nick

[toc] | [prev] | [next] | [standalone]

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

On Thu, Nov 24, 2016 at 4:40 PM, Nicholas Piggin <npiggin@gmail.com> wrote:
>>
>> Yes, manual "marking" is never going to be a viable solution.
>
> I guess it really depends on how exactly you want to use it. For distros
> that do stable ABI but rarely may have to break something for security
> reasons, it should work and give exact control.

No. Because nobody else will care, so unless it's like a single symbol
or something, it will just be a maintenance nightmare.

> What else do people *actually* use it for? Preventing mismatched modules
> when .git version is not attached and release version of the kernel has
> not been bumped. Is that it?

It used to be very useful for avoiding loading stale modules and then
wasting days on debugging something that wasn't the case when you had
forgotten to do "make modules_install". Change some subtle internal
ABI issue (add/remove a parameter, whatever) and it would really help.

These days, for me, LOCALVERSION_AUTO and module signing are what I
personally tend to use.

The modversions stuff may just be too painful to bother with. Very few
people probably use it, and the ones that do likely don't have any
overriding reason why.

So I'd personally be ok with just saying "let's disable it for now",
and see if anybody even notices and cares, and then has a good enough
explanation of why. It's entirely possible that most users are "I
enabled it ten years ago, I didn't even realize it was still in my
defconfig".

                    Linus

[toc] | [prev] | [next] | [standalone]

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

On Fri, 25 Nov 2016 10:00:46 -0800
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Thu, Nov 24, 2016 at 4:40 PM, Nicholas Piggin <npiggin@gmail.com> wrote:
> >>
> >> Yes, manual "marking" is never going to be a viable solution.  
> >
> > I guess it really depends on how exactly you want to use it. For distros
> > that do stable ABI but rarely may have to break something for security
> > reasons, it should work and give exact control.  
> 
> No. Because nobody else will care, so unless it's like a single symbol
> or something, it will just be a maintenance nightmare.

Yeah that's true, and as I realized a distro can rename a symbol if they
make incompatible changes which happens very rarely. Avoids having to
carry some whole infrastructure upstream for it.

> 
> > What else do people *actually* use it for? Preventing mismatched modules
> > when .git version is not attached and release version of the kernel has
> > not been bumped. Is that it?  
> 
> It used to be very useful for avoiding loading stale modules and then
> wasting days on debugging something that wasn't the case when you had
> forgotten to do "make modules_install". Change some subtle internal
> ABI issue (add/remove a parameter, whatever) and it would really help.
> 
> These days, for me, LOCALVERSION_AUTO and module signing are what I
> personally tend to use.
> 
> The modversions stuff may just be too painful to bother with. Very few
> people probably use it, and the ones that do likely don't have any
> overriding reason why.
> 
> So I'd personally be ok with just saying "let's disable it for now",
> and see if anybody even notices and cares, and then has a good enough
> explanation of why. It's entirely possible that most users are "I
> enabled it ten years ago, I didn't even realize it was still in my
> defconfig".

That sounds good. Should we try to get 4.9 working (which we could
do relatively easily with a few arch reverts), and then disable
modversions for 4.10? (at which point we can un-revert Al's arch
patches)

Thanks,
Nick

[toc] | [prev] | [next] | [standalone]

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

[Multipart message — attachments visible in raw view] — view raw

[I've had to guess at the cc list for this, because we no longer have
mail archives that preserve them.]

On Fri, 2016-11-25 at 10:01 -0800, Linus Torvalds wrote:
> On Thu, Nov 24, 2016 at 4:40 PM, Nicholas Piggin <npiggin@gmail.com> wrote:
> > > 
> > > Yes, manual "marking" is never going to be a viable solution.
> > 
> > I guess it really depends on how exactly you want to use it. For distros
> > that do stable ABI but rarely may have to break something for security
> > reasons, it should work and give exact control.

This is roughly how Debian handles the kernel module ABI during a
stable release.

> No. Because nobody else will care, so unless it's like a single symbol
> or something, it will just be a maintenance nightmare.

I agree with this.  We can explicitly "version" individual symbols
anyway by doing something like:

-int foo(void);
+#define foo foo_2
+int foo_2(int);

> > What else do people *actually* use it for? Preventing mismatched modules
> > when .git version is not attached and release version of the kernel has
> > not been bumped. Is that it?
> 
> It used to be very useful for avoiding loading stale modules and then
> wasting days on debugging something that wasn't the case when you had
> forgotten to do "make modules_install". Change some subtle internal
> ABI issue (add/remove a parameter, whatever) and it would really help.
> 
> These days, for me, LOCALVERSION_AUTO and module signing are what I
> personally tend to use.
>
> The modversions stuff may just be too painful to bother with. Very few
> people probably use it, and the ones that do likely don't have any
> overriding reason why.
[...]

Debian has some strong reasons:

1. Changing the release string requires any out-of-tree modules to be
upgraded (at least rebuilt) on end-user systems.  So we try to avoid
doing that during the lifetime of a stable release, i.e. we don't let
the release string change.  Also, the release string is reflected in
package names (e.g. linux-image-4.8.0-1-amd64), and introducing new
package names requires manual approval by the Debian archive team.

2. We want to allow ABI breaks for "internal" symbols used only by in-
tree modules, as those breaks will be resolved by rebooting to complete
the upgrade.  But we need a run-time check to prevent loading an
incompatible module before the reboot.

3. So far as I can see, module signing doesn't work for a distribution
kernel with out-of-tree modules as there has to be a trust path from a
built-in certificate to the module signing certificate.  So signature
enforcement will have to be disabled on systems that use out-of-tree
modules, thus it's not a substitute for modversions.

We expect Linux 4.9 to be the basis for a longterm stable branch and on
that basis intend to include it in the next Debian stable release. 
Even if the decision is to get rid of modversions, it would be very
helpful if they could be revived for 4.9 so that we have some time to
adapt our packaging practices to work without them in future.

Ben.

-- 
Ben Hutchings
Theory and practice are closer in theory than in practice.
                                - John Levine, moderator of
comp.compilers

[toc] | [prev] | [next] | [standalone]

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

On Tue, 29 Nov 2016 01:15:48 +0000
Ben Hutchings <ben@decadent.org.uk> wrote:

> [I've had to guess at the cc list for this, because we no longer have
> mail archives that preserve them.]

You got it about right.

> On Fri, 2016-11-25 at 10:01 -0800, Linus Torvalds wrote:
> > On Thu, Nov 24, 2016 at 4:40 PM, Nicholas Piggin <npiggin@gmail.com> wrote:  
> > > > 
> > > > Yes, manual "marking" is never going to be a viable solution.  
> > > 
> > > I guess it really depends on how exactly you want to use it. For distros
> > > that do stable ABI but rarely may have to break something for security
> > > reasons, it should work and give exact control.  
> 
> This is roughly how Debian handles the kernel module ABI during a
> stable release.
> 
> > No. Because nobody else will care, so unless it's like a single symbol
> > or something, it will just be a maintenance nightmare.  
> 
> I agree with this.  We can explicitly "version" individual symbols
> anyway by doing something like:
> 
> -int foo(void);
> +#define foo foo_2
> +int foo_2(int);

Yeah... Benefit being it's very simple and everybody can see exactly
what it does and knows how it will work.

> 
> > > What else do people *actually* use it for? Preventing mismatched modules
> > > when .git version is not attached and release version of the kernel has
> > > not been bumped. Is that it?  
> > 
> > It used to be very useful for avoiding loading stale modules and then
> > wasting days on debugging something that wasn't the case when you had
> > forgotten to do "make modules_install". Change some subtle internal
> > ABI issue (add/remove a parameter, whatever) and it would really help.
> > 
> > These days, for me, LOCALVERSION_AUTO and module signing are what I
> > personally tend to use.
> >
> > The modversions stuff may just be too painful to bother with. Very few
> > people probably use it, and the ones that do likely don't have any
> > overriding reason why.  
> [...]
> 
> Debian has some strong reasons:
> 
> 1. Changing the release string requires any out-of-tree modules to be
> upgraded (at least rebuilt) on end-user systems.  So we try to avoid
> doing that during the lifetime of a stable release, i.e. we don't let
> the release string change.  Also, the release string is reflected in
> package names (e.g. linux-image-4.8.0-1-amd64), and introducing new
> package names requires manual approval by the Debian archive team.

This is something I've noticed. Would it be better if the module loader
ignores the kernel version and instead used some internal ABI version
string to check against? Otherwise (AFAICT) you always have 4.8.0 versions
despite being 4.8.7 kernel, and you can't upgrade a point release without
overwriting your old kernel and modules.

That is something we could potentially replace modversions with. It would
be a far more reasonable complexity to carry for downstream distros than
modversions. Though not something we can add for 4.9.

> 2. We want to allow ABI breaks for "internal" symbols used only by in-
> tree modules, as those breaks will be resolved by rebooting to complete
> the upgrade.  But we need a run-time check to prevent loading an
> incompatible module before the reboot.
> 
> 3. So far as I can see, module signing doesn't work for a distribution
> kernel with out-of-tree modules as there has to be a trust path from a
> built-in certificate to the module signing certificate.  So signature
> enforcement will have to be disabled on systems that use out-of-tree
> modules, thus it's not a substitute for modversions.
> 
> We expect Linux 4.9 to be the basis for a longterm stable branch and on
> that basis intend to include it in the next Debian stable release. 
> Even if the decision is to get rid of modversions, it would be very
> helpful if they could be revived for 4.9 so that we have some time to
> adapt our packaging practices to work without them in future.

It would be nice to get upstream to the point where 4.9 modversions
works if you just patch out depends BROKEN. That would require reverting
a few more of Al's arch patches.

Then in 4.10 we can re-add all those arch patches (which are less
controversial without the asm-prototypes.h workaround), and implement a
simple stable ABI version string check, and then in 4.11 we can remove
modversions.

Thanks,
Nick

[toc] | [prev] | [next] | [standalone]

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

Dne 29.11.2016 v 03:31 Nicholas Piggin napsal(a):
> On Tue, 29 Nov 2016 01:15:48 +0000
> Ben Hutchings <ben@decadent.org.uk> wrote:
> 
>> [I've had to guess at the cc list for this, because we no longer have
>> mail archives that preserve them.]
> 
> You got it about right.
> 
>> On Fri, 2016-11-25 at 10:01 -0800, Linus Torvalds wrote:
>>> On Thu, Nov 24, 2016 at 4:40 PM, Nicholas Piggin <npiggin@gmail.com> wrote:  
>>>>>
>>>>> Yes, manual "marking" is never going to be a viable solution.  
>>>>
>>>> I guess it really depends on how exactly you want to use it. For distros
>>>> that do stable ABI but rarely may have to break something for security
>>>> reasons, it should work and give exact control.  
>>
>> This is roughly how Debian handles the kernel module ABI during a
>> stable release.
>>
>>> No. Because nobody else will care, so unless it's like a single symbol
>>> or something, it will just be a maintenance nightmare.  
>>
>> I agree with this.  We can explicitly "version" individual symbols
>> anyway by doing something like:
>>
>> -int foo(void);
>> +#define foo foo_2
>> +int foo_2(int);
> 
> Yeah... Benefit being it's very simple and everybody can see exactly
> what it does and knows how it will work.
> 
>>
>>>> What else do people *actually* use it for? Preventing mismatched modules
>>>> when .git version is not attached and release version of the kernel has
>>>> not been bumped. Is that it?  
>>>
>>> It used to be very useful for avoiding loading stale modules and then
>>> wasting days on debugging something that wasn't the case when you had
>>> forgotten to do "make modules_install". Change some subtle internal
>>> ABI issue (add/remove a parameter, whatever) and it would really help.
>>>
>>> These days, for me, LOCALVERSION_AUTO and module signing are what I
>>> personally tend to use.
>>>
>>> The modversions stuff may just be too painful to bother with. Very few
>>> people probably use it, and the ones that do likely don't have any
>>> overriding reason why.  
>> [...]
>>
>> Debian has some strong reasons:

I guess many distros have similar reasons.


>> 1. Changing the release string requires any out-of-tree modules to be
>> upgraded (at least rebuilt) on end-user systems.  So we try to avoid
>> doing that during the lifetime of a stable release, i.e. we don't let
>> the release string change.  Also, the release string is reflected in
>> package names (e.g. linux-image-4.8.0-1-amd64), and introducing new
>> package names requires manual approval by the Debian archive team.
> 
> This is something I've noticed. Would it be better if the module loader
> ignores the kernel version and instead used some internal ABI version
> string to check against? Otherwise (AFAICT) you always have 4.8.0 versions
> despite being 4.8.7 kernel, and you can't upgrade a point release without
> overwriting your old kernel and modules.

The thing is - to maintain an ABI version string, you need some level of
certainty that two given ABIs are really interchangeable. Which means
you need to check whether the symbols _and_ types exposed are unchanged.
Which is a thing that genksyms, the tool behind CONFIG_MODVERSIONS, does
quite well. So yes, you could do a testbuild with CONFIG_MODVERSIONS=y
and a production build with some global ABI string, but what's the point
then.


> It would be nice to get upstream to the point where 4.9 modversions
> works if you just patch out depends BROKEN. That would require reverting
> a few more of Al's arch patches.
> 
> Then in 4.10 we can re-add all those arch patches (which are less
> controversial without the asm-prototypes.h workaround), and implement a
> simple stable ABI version string check, and then in 4.11 we can remove
> modversions.

I'd rather change the kconfig to

depends on BROKEN || <archs that have asm/asm-prototypes.h>

and eventuallly remove the dependency again. PPC has the header already,
so it can be added right away. I do not know why the x86 patch has not
been merged yet.

Michal

[toc] | [prev] | [next] | [standalone]

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

On Mon, Nov 28, 2016 at 5:15 PM, Ben Hutchings <ben@decadent.org.uk> wrote:
>>
>> The modversions stuff may just be too painful to bother with. Very few
>> people probably use it, and the ones that do likely don't have any
>> overriding reason why.
> [...]
>
> Debian has some strong reasons:

Honestly, I'd just like to see actual real patches from people who
care about this.

The reason I disabled it entirely was simply that the discussions had
been going on forever, but nobody actually seemed to care enough to
just fix the damn thing. There was all the _noise_ about "look, here's
a patch", but nothing got sent to maintainers and actually actively
pushed as a "this fixes a regression".

At some point I just get fed up and say "this isn't worth the hot air
and endless pointless blathering".

What is the actual exact failure with MODVERSIONS today? IOW, if you
just remove the "broken", is it actually broken, and why? Because it
does work for me, I just got really tired of hearing about it, and
assuming it's just some broken toolchain or other case that I just
don't hit.

So somebody send me a minimal patch that is

 (a) tested
 (b) explains it
 (c) obvious

and I'll happily re-enable modversions.

                    Linus

[toc] | [prev] | [next] | [standalone]

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

On Mon, Nov 28, 2016 at 08:08:57PM -0800, Linus Torvalds wrote:
> On Mon, Nov 28, 2016 at 5:15 PM, Ben Hutchings <ben@decadent.org.uk> wrote:
> >>
> >> The modversions stuff may just be too painful to bother with. Very few
> >> people probably use it, and the ones that do likely don't have any
> >> overriding reason why.
> > [...]
> >
> > Debian has some strong reasons:
> 
> Honestly, I'd just like to see actual real patches from people who
> care about this.
> 
> The reason I disabled it entirely was simply that the discussions had
> been going on forever, but nobody actually seemed to care enough to
> just fix the damn thing. There was all the _noise_ about "look, here's
> a patch", but nothing got sent to maintainers and actually actively
> pushed as a "this fixes a regression".

Here's some history:
The day of -rc1, multiple people immediately reported the breakage; it was
quickly found out that reverting 784d5699eddc fixes it.  A "going forward"
patch has been posted but was insufficient; when the real devs went to bed
the last message was
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1250370.html
which ends with instructions and "Care to do a patch for x86?".

Then a random person (me) did the legwork, gathered affected symbols, wrote
and tested the x86 patch.  It was then tested by multiple people; Arnd
Bergmann wrote the ARM equivalent.  Whenever a new lkml thread reporting the
breakage popped up, we pointed people to the patches and everyone was happy.
As for upstreaming, there was a delay because Michal Marek was on vacation.

Michal returned and sent you the pull request, you merged it as 04e36857 on
Nov 18.  For some reason the per-arch pieces were excluded; I was instructed
to send my part to x86 maintainers.

I did so; the patch later got a better description by Nick and a bunch of
Tested-by -- but alas, nary a comment or action from x86 guys, despite
pings/resends (last one: https://lkml.org/lkml/2016/11/23/706).  I guess I'm
lacking the secret handshake or something -- thus, it looks like it's my
fault, the rest of you can be blamed mostly for letting a
not-a-real-kernel-dev unsupervised.

On Nov 24 finally Ingo responded, the discussion ended with you marking
modversions as BROKEN.


> So somebody send me a minimal patch that is
> 
>  (a) tested
>  (b) explains it
>  (c) obvious
> 
> and I'll happily re-enable modversions.

Not sure whether you guys want to revert or to go forward.  If the latter,
my piece handles x86, Arnd's ARM.  Powerpc already has the needed bits in
mainline.  I've just tried arm64 -- despite same toolchain versions as
failing x86, with CONFIG_MODVERSIONS=y it loaded a bunch of modules fine so
it appears no arch bits are needed there.  My uneducated guess is that most
other architectures should be fine without special handling, too.  It'd be
nice if someone with an actual clue could confirm.


Meow!
-- 
The bill declaring Jesus as the King of Poland fails to specify whether
the addition is at the top or end of the list of kings.  What should the
historians do?

[toc] | [prev] | [next] | [standalone]

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

* Adam Borowski <kilobyte@angband.pl> wrote:

> Here's some history:
> The day of -rc1, multiple people immediately reported the breakage; it was
> quickly found out that reverting 784d5699eddc fixes it.  A "going forward"
> patch has been posted but was insufficient; when the real devs went to bed
> the last message was
> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1250370.html
> which ends with instructions and "Care to do a patch for x86?".
> 
> Then a random person (me) did the legwork, gathered affected symbols, wrote
> and tested the x86 patch.  It was then tested by multiple people; Arnd
> Bergmann wrote the ARM equivalent.  Whenever a new lkml thread reporting the
> breakage popped up, we pointed people to the patches and everyone was happy.
> As for upstreaming, there was a delay because Michal Marek was on vacation.
> 
> Michal returned and sent you the pull request, you merged it as 04e36857 on
> Nov 18.  For some reason the per-arch pieces were excluded; I was instructed
> to send my part to x86 maintainers.
> 
> I did so; the patch later got a better description by Nick and a bunch of
> Tested-by -- but alas, nary a comment or action from x86 guys, despite
> pings/resends (last one: https://lkml.org/lkml/2016/11/23/706).  I guess I'm
> lacking the secret handshake or something -- thus, it looks like it's my
> fault, the rest of you can be blamed mostly for letting a
> not-a-real-kernel-dev unsupervised.

My part of the story is easy to explain: the reason I skipped the 11/23 patch was 
because it was tagged 'kbuild' and because the commit that broke it was never 
acked by (or was upstreamed via) the x86 maintainers - we never upstreamed any 
modversions changes in the past AFAIR - so I assumed it would be handled via 
whatever path got the breakage upstream (turns out it was via the VFS tree?),
or via the kbuild tree.

> On Nov 24 finally Ingo responded, the discussion ended with you marking 
> modversions as BROKEN.

Yeah, that was when my internal timer ran out: modversions breakage was reported 
against -rc1 already and it still wasn't working (a seemingly working kernel build 
resulted in an unbootable system) - due to the timeline and confusion you 
explained.

I totally agree with marking it BROKEN: it was the simplest, most robust way to 
fix it and nobody seemed to be owning the modversions feature.

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

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

On Tue, Nov 29, 2016 at 02:29:54PM +0100, Ingo Molnar wrote:
> * Adam Borowski <kilobyte@angband.pl> wrote:
> 
> > Here's some history:
> > The day of -rc1, multiple people immediately reported the breakage; it was
> > quickly found out that reverting 784d5699eddc fixes it.  A "going forward"
> > patch has been posted but was insufficient; when the real devs went to bed
[...]
> > For some reason the per-arch pieces were excluded; I was instructed
> > to send my part to x86 maintainers.
> > 
> > I did so; the patch later got a better description by Nick and a bunch of
> > Tested-by -- but alas, nary a comment or action from x86 guys, despite
> > pings/resends (last one: https://lkml.org/lkml/2016/11/23/706).  I guess I'm
> > lacking the secret handshake or something -- thus, it looks like it's my
> > fault, the rest of you can be blamed mostly for letting a
> > not-a-real-kernel-dev unsupervised.
> 
> My part of the story is easy to explain: the reason I skipped the 11/23 patch was 
> because it was tagged 'kbuild' and because the commit that broke it was never 
> acked by (or was upstreamed via) the x86 maintainers - we never upstreamed any 
> modversions changes in the past AFAIR - so I assumed it would be handled via 
> whatever path got the breakage upstream (turns out it was via the VFS tree?),
> or via the kbuild tree.

The problematic merge was 84d6984 (it brought in 22823ab4^..590abbdd).
Interesting commits have "$ARCH: move exports to definitions" in their
subjects, they indeed did not go through arch trees.

> > On Nov 24 finally Ingo responded, the discussion ended with you marking 
> > modversions as BROKEN.
> 
> Yeah, that was when my internal timer ran out: modversions breakage was reported 
> against -rc1 already and it still wasn't working (a seemingly working kernel build 
> resulted in an unbootable system) - due to the timeline and confusion you 
> explained.
> 
> I totally agree with marking it BROKEN: it was the simplest, most robust way to 
> fix it and nobody seemed to be owning the modversions feature.

Per Ben Hutchings' objection, it doesn't look like BROKEN is an option at
least for 4.9 -- even if mainline leaves it as is, distro maintainers would
need to do the work themselves.

Architectures that look like they could be affected:
    x86 alpha m68k s390 arm ppc sparc ia64
(this list might be incomplete!).

Of these, ppc and arm are already fixed, x86 is in this thread,
arm64 is not on the list which explains why it works for me.  No idea about
the rest.


Meow!
-- 
The bill declaring Jesus as the King of Poland fails to specify whether
the addition is at the top or end of the list of kings.  What should the
historians do?

[toc] | [prev] | [next] | [standalone]

#1532310 — [PATCH] x86/kbuild: enable modversions for symbols exported from asm

Commit 4efca4ed ("kbuild: modversions for EXPORT_SYMBOL() for asm") adds
modversion support for symbols exported from asm files. Architectures
must include C-style declarations for those symbols in asm/asm-prototypes.h
in order for them to be versioned.

Add these declarations for x86, and an architecture-independent file that
can be used for common symbols.

User impact: kernels may fail to load modules at all when
CONFIG_MODVERSIONS=y.

Signed-off-by: Adam Borowski <kilobyte@angband.pl>
Tested-by: Kalle Valo <kvalo@codeaurora.org>
Acked-by: Nicholas Piggin <npiggin@gmail.com>
Tested-by: Peter Wu <peter@lekensteyn.nl>
Tested-by: Oliver Hartkopp <socketcan@hartkopp.net>
---

> So somebody send me a minimal patch that is
>
>  (a) tested

By many people.

>  (b) explains it

The actual logic is in 4efca4ed0.  It wants C prototypes defined in
asm/asm-prototypes.h that lists symbols defined in assembly -- genksyms
knows only how to read C code.

>  (c) obvious

To be honest I don't quite understand what's the real gain over code that
was removed by 784d5699eddc, this mostly brings the symbols back.  But
that's for people wiser than me to explain.

> and I'll happily re-enable modversions.


The powerpc counterpart to this patch is in mainline as 9e5f688, although a
file by that name already existed.

As for arm, it looks like it was handled the other way by 8478132.


diff --git a/arch/x86/include/asm/asm-prototypes.h b/arch/x86/include/asm/asm-prototypes.h
new file mode 100644
index 0000000..ae87224
--- /dev/null
+++ b/arch/x86/include/asm/asm-prototypes.h
@@ -0,0 +1,12 @@
+#include <asm/ftrace.h>
+#include <asm/uaccess.h>
+#include <asm/string.h>
+#include <asm/page.h>
+#include <asm/checksum.h>
+
+#include <asm-generic/asm-prototypes.h>
+
+#include <asm/page.h>
+#include <asm/pgtable.h>
+#include <asm/special_insns.h>
+#include <asm/preempt.h>
diff --git a/include/asm-generic/asm-prototypes.h b/include/asm-generic/asm-prototypes.h
new file mode 100644
index 0000000..df13637
--- /dev/null
+++ b/include/asm-generic/asm-prototypes.h
@@ -0,0 +1,7 @@
+#include <linux/bitops.h>
+extern void *__memset(void *, int, __kernel_size_t);
+extern void *__memcpy(void *, const void *, __kernel_size_t);
+extern void *__memmove(void *, const void *, __kernel_size_t);
+extern void *memset(void *, int, __kernel_size_t);
+extern void *memcpy(void *, const void *, __kernel_size_t);
+extern void *memmove(void *, const void *, __kernel_size_t);
-- 
2.10.2

[toc] | [prev] | [next] | [standalone]

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

[Multipart message — attachments visible in raw view] — view raw

On Nov 29, 2016 5:51 AM, "Adam Borowski" <kilobyte@angband.pl> wrote:
>

> >
> >  (a) tested
>
> By many people.

No.

I've tested the build *without* this, and it works fine.

> >  (b) explains it
>
> The actual logic is in 4efca4ed0.  It wants C prototypes defined in
> asm/asm-prototypes.h that lists symbols defined in assembly -- genksyms
> knows only how to read C code.

See above. I'm not taking more random patches that "fix" this when it's not
broken for me. Not without very explicit explanations of why that patch is
still needed for others.

I suspect one of the other patches already fixed is for x86.

     Linus

[toc] | [prev] | [next] | [standalone]

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

Dne 29.11.2016 v 16:27 Linus Torvalds napsal(a):
> On Nov 29, 2016 5:51 AM, "Adam Borowski" <kilobyte@angband.pl
> <mailto:kilobyte@angband.pl>> wrote:
>>
> 
>> >
>> >  (a) tested
>>
>> By many people.
> 
> No.
> 
> I've tested the build *without* this, and it works fine.
> 
>> >  (b) explains it
>>
>> The actual logic is in 4efca4ed0.  It wants C prototypes defined in
>> asm/asm-prototypes.h that lists symbols defined in assembly -- genksyms
>> knows only how to read C code.
> 
> See above. I'm not taking more random patches that "fix" this when it's
> not broken for me. Not without very explicit explanations of why that
> patch is still needed for others.

The original and easily observable bug is that were are not generating
symbol checksums for the asm-exported symbols, so they default to 0.
This can be seen e.g. in the Module.symvers file. This seemed like a
minor issue, because with the functions written in asm, the type
checking is rather weak (this has been the case even before Al's
patches). However, there is another bug that with _some_ toolchains /
architectures, the checksums do not default to 0, but they are simply
missing in the ___kcrctab* sections and the module loader complains. We
can of course research into the details of the second bug, but we
already know that we are not generating the checksums while we should be.

Michal

[toc] | [prev] | [next] | [standalone]

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

On Tue, Nov 29, 2016 at 8:03 AM, Michal Marek <mmarek@suse.com> wrote:
>
> The original and easily observable bug is that were are not generating
> symbol checksums for the asm-exported symbols, so they default to 0.
> This can be seen e.g. in the Module.symvers file. This seemed like a
> minor issue, because with the functions written in asm, the type
> checking is rather weak (this has been the case even before Al's
> patches). However, there is another bug that with _some_ toolchains /
> architectures, the checksums do not default to 0, but they are simply
> missing in the ___kcrctab* sections and the module loader complains. We
> can of course research into the details of the second bug, but we
> already know that we are not generating the checksums while we should be.

So let's just say that "toolchain is buggy" and make a missing kcrctab
entry mean zero (or mean "matches anything"). And just shut up the
warning.

I do *not* want to add random bandaids for something like a broken
toolchain issue when I'd really rather just delete the feature.

                     Linus

[toc] | [prev] | [next] | [standalone]

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

[Multipart message — attachments visible in raw view] — view raw

On Tue, 2016-11-29 at 08:17 -0800, Linus Torvalds wrote:
> > On Tue, Nov 29, 2016 at 8:03 AM, Michal Marek <mmarek@suse.com> wrote:
> > 
> > The original and easily observable bug is that were are not generating
> > symbol checksums for the asm-exported symbols, so they default to 0.
> > This can be seen e.g. in the Module.symvers file. This seemed like a
> > minor issue, because with the functions written in asm, the type
> > checking is rather weak (this has been the case even before Al's
> > patches). However, there is another bug that with _some_ toolchains /
> > architectures, the checksums do not default to 0, but they are simply
> > missing in the ___kcrctab* sections and the module loader complains. We
> > can of course research into the details of the second bug, but we
> > already know that we are not generating the checksums while we should be.
> 
> So let's just say that "toolchain is buggy" and make a missing kcrctab
> entry mean zero (or mean "matches anything"). And just shut up the
> warning.
> 
> I do *not* want to add random bandaids for something like a broken
> toolchain issue when I'd really rather just delete the feature.

If the modversion is missing then the fallback should be to a full
vermagic match, i.e. including the release string.  Something like
this (untested):

diff --git a/init/Kconfig b/init/Kconfig
index c4fbc1e55c25..34407f15e6d3 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1945,7 +1945,6 @@ config MODULE_FORCE_UNLOAD
 
 config MODVERSIONS
 	bool "Module versioning support"
-	depends on BROKEN
 	help
 	  Usually, you have to use modules compiled with your kernel.
 	  Saying Y here makes it sometimes possible to use modules
diff --git a/kernel/module.c b/kernel/module.c
index f57dd63186e6..78d61ae50bc5 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -296,6 +296,12 @@ int unregister_module_notifier(struct notifier_block *nb)
 }
 EXPORT_SYMBOL(unregister_module_notifier);
 
+enum {
+	MAGIC_NO_MATCH,
+	MAGIC_MATCH_NEED_CRC,
+	MAGIC_MATCH_EXACT
+};
+
 struct load_info {
 	Elf_Ehdr *hdr;
 	unsigned long len;
@@ -305,6 +311,7 @@ struct load_info {
 	struct _ddebug *debug;
 	unsigned int num_debug;
 	bool sig_ok;
+	int magic_match;
 #ifdef CONFIG_KALLSYMS
 	unsigned long mod_kallsyms_init_off;
 #endif
@@ -1268,13 +1275,14 @@ static unsigned long maybe_relocated(unsigned long crc,
 	return crc;
 }
 
-static int check_version(Elf_Shdr *sechdrs,
-			 unsigned int versindex,
+static int check_version(const struct load_info *info,
 			 const char *symname,
 			 struct module *mod,
 			 const unsigned long *crc,
 			 const struct module *crc_owner)
 {
+	Elf_Shdr *sechdrs = info->sechdrs;
+	unsigned int versindex = info->index.vers;
 	unsigned int i, num_versions;
 	struct modversion_info *versions;
 
@@ -1294,6 +1302,10 @@ static int check_version(Elf_Shdr *sechdrs,
 		if (strcmp(versions[i].name, symname) != 0)
 			continue;
 
+		/* Ignore dummy zero CRC */
+		if (versions[i].crc == 0)
+			break;
+
 		if (versions[i].crc == maybe_relocated(*crc, crc_owner))
 			return 1;
 		pr_debug("Found checksum %lX vs module %lX\n",
@@ -1301,6 +1313,9 @@ static int check_version(Elf_Shdr *sechdrs,
 		goto bad_version;
 	}
 
+	if (info->magic_match == MAGIC_MATCH_EXACT)
+		return 1;
+
 	pr_warn("%s: no symbol version for %s\n", mod->name, symname);
 	return 0;
 
@@ -1310,8 +1325,7 @@ static int check_version(Elf_Shdr *sechdrs,
 	return 0;
 }
 
-static inline int check_modstruct_version(Elf_Shdr *sechdrs,
-					  unsigned int versindex,
+static inline int check_modstruct_version(const struct load_info *info,
 					  struct module *mod)
 {
 	const unsigned long *crc;
@@ -1327,24 +1341,24 @@ static inline int check_modstruct_version(Elf_Shdr *sechdrs,
 		BUG();
 	}
 	preempt_enable();
-	return check_version(sechdrs, versindex,
+	return check_version(info,
 			     VMLINUX_SYMBOL_STR(module_layout), mod, crc,
 			     NULL);
 }
 
-/* First part is kernel version, which we ignore if module has crcs. */
-static inline int same_magic(const char *amagic, const char *bmagic,
-			     bool has_crcs)
+/* First part is kernel version, which can be ignored if module has crcs. */
+static inline int compare_magic(const char *amagic, const char *bmagic)
 {
-	if (has_crcs) {
-		amagic += strcspn(amagic, " ");
-		bmagic += strcspn(bmagic, " ");
-	}
-	return strcmp(amagic, bmagic) == 0;
+	if (strcmp(amagic, bmagic) == 0)
+		return MAGIC_MATCH_EXACT;
+
+	amagic += strcspn(amagic, " ");
+	bmagic += strcspn(bmagic, " ");
+	return strcmp(amagic, bmagic) == 0 ? MAGIC_MATCH_NEED_CRC : MAGIC_NO_MATCH;
 }
+
 #else
-static inline int check_version(Elf_Shdr *sechdrs,
-				unsigned int versindex,
+static inline int check_version(const struct load_info *info,
 				const char *symname,
 				struct module *mod,
 				const unsigned long *crc,
@@ -1353,17 +1367,15 @@ static inline int check_version(Elf_Shdr *sechdrs,
 	return 1;
 }
 
-static inline int check_modstruct_version(Elf_Shdr *sechdrs,
-					  unsigned int versindex,
+static inline int check_modstruct_version(const struct load_info *info,
 					  struct module *mod)
 {
 	return 1;
 }
 
-static inline int same_magic(const char *amagic, const char *bmagic,
-			     bool has_crcs)
+static inline int compare_magic(const char *amagic, const char *bmagic)
 {
-	return strcmp(amagic, bmagic) == 0;
+	return strcmp(amagic, bmagic) == 0 ? MAGIC_MATCH_EXACT : MAGIC_NO_MATCH;
 }
 #endif /* CONFIG_MODVERSIONS */
 
@@ -1390,8 +1402,7 @@ static const struct kernel_symbol *resolve_symbol(struct module *mod,
 	if (!sym)
 		goto unlock;
 
-	if (!check_version(info->sechdrs, info->index.vers, name, mod, crc,
-			   owner)) {
+	if (!check_version(info, name, mod, crc, owner)) {
 		sym = ERR_PTR(-EINVAL);
 		goto getname;
 	}
@@ -2936,7 +2947,7 @@ static struct module *setup_load_info(struct load_info *info, int flags)
 	info->index.pcpu = find_pcpusec(info);
 
 	/* Check module struct version now, before we try to use module. */
-	if (!check_modstruct_version(info->sechdrs, info->index.vers, mod))
+	if (!check_modstruct_version(info, mod))
 		return ERR_PTR(-ENOEXEC);
 
 	return mod;
@@ -2952,13 +2963,19 @@ static int check_modinfo(struct module *mod, struct load_info *info, int flags)
 
 	/* This is allowed: modprobe --force will invalidate it. */
 	if (!modmagic) {
+		info->magic_match = MAGIC_NO_MATCH;
 		err = try_to_force_load(mod, "bad vermagic");
 		if (err)
 			return err;
-	} else if (!same_magic(modmagic, vermagic, info->index.vers)) {
-		pr_err("%s: version magic '%s' should be '%s'\n",
-		       mod->name, modmagic, vermagic);
-		return -ENOEXEC;
+	} else {
+		info->magic_match = compare_magic(modmagic, vermagic);
+		if (info->magic_match == MAGIC_NO_MATCH ||
+		    (info->magic_match == MAGIC_MATCH_NEED_CRC &&
+		     !info->index.vers)) {
+			pr_err("%s: version magic '%s' should be '%s'\n",
+			       mod->name, modmagic, vermagic);
+			return -ENOEXEC;
+		}
 	}
 
 	if (!get_modinfo(info, "intree")) {
--- END ---

Ben.

-- 
Ben Hutchings
Theory and practice are closer in theory than in practice.
                                - John Levine, moderator of comp.compilers

[toc] | [prev] | [next] | [standalone]

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

On Tue, Nov 29, 2016 at 11:57 AM, Ben Hutchings <ben@decadent.org.uk> wrote:
>
> If the modversion is missing then the fallback should be to a full
> vermagic match, i.e. including the release string.  Something like
> this (untested):

This really seems way too complicated for this situation.

And it's wrong too. The whole point of modversions was that you didn't
want to do the full version check.

We already know there were *some* crc's (we checked that at the top of
check_version(), but we've also checked it in "same_magic()" - it's
what makes us ignore the exact version number), but this particular
symbol doesn't have a crc. Just let it through, because we have bugs
in binutils.

So your extra complexity logic seems actively wrong. It makes
MODVERSIONS not work at all, rather than limp along. You're better off
just not having MODVERSIONS.

               Linus

[toc] | [prev] | [next] | [standalone]

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

On Wed, Nov 30, 2016 at 10:18 AM, Nicholas Piggin <npiggin@gmail.com> 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..

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

[toc] | [prev] | [next] | [standalone]

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

[Multipart message — attachments visible in raw view] — view raw

On Wed, 2016-11-30 at 10:40 -0800, Linus Torvalds wrote:
> > On Wed, Nov 30, 2016 at 10:18 AM, Nicholas Piggin <npiggin@gmail.com> 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,

The last time I looked, RHEL and SLE did.  They change the release
string for each new kernel version, but they will copy/link old out-of-
tree modules into the new version's "weak-updates" module subdirectory
if the symbol versions still match.

> 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..

Allowing the vermagic to be changed separately doesn't help us, as we
already control the release string.  If we were to change some of the
module tools to consider vermagic then it would allow us to report the
full version in the release string while not forcing rebuilds on every
kernel upgrade - but that's not a pressing problem.

One thing that could work for us would be:

- Stricter version matching for in-tree modules (maybe some extra
  part in vermagic that is skipped for out-of-tree modules)
- Ability to blacklist use of a symbol, or all symbols in a module,
  by out-of-tree modules

where the blacklist would be a matter of distribution policy.  But this
would still require a fair amount of work by someone, and I doubt you'd
want this upstream.

> 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.

If this was just about non-free modules I wouldn't care.  There are
also many freely licenced out-of-tree modules that for various reasons
don't get submitted or accepted upstream; also backports of new or
updated drivers.

Ben.

-- 
Ben Hutchings
Never attribute to conspiracy what can adequately be explained by
stupidity.

[toc] | [prev] | [next] | [standalone]

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

On Wed, 30 Nov 2016 21:33:01 +0000
Ben Hutchings <ben@decadent.org.uk> wrote:

> On Wed, 2016-11-30 at 10:40 -0800, Linus Torvalds wrote:
> > > On Wed, Nov 30, 2016 at 10:18 AM, Nicholas Piggin <npiggin@gmail.com> 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,  
> 
> The last time I looked, RHEL and SLE did.  They change the release
> string for each new kernel version, but they will copy/link old out-of-
> tree modules into the new version's "weak-updates" module subdirectory
> if the symbol versions still match.
> 
> > 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..  
> 
> Allowing the vermagic to be changed separately doesn't help us, as we
> already control the release string.  If we were to change some of the
> module tools to consider vermagic then it would allow us to report the
> full version in the release string while not forcing rebuilds on every
> kernel upgrade - but that's not a pressing problem.

Okay, but existing modversions AFAIKS does not solve your problems described
in yor your earlier mail either. Modversions hardly catches ABI breakage at
all, you can't rely on it that way. It's far more likely that some structure
size changes deep in the kernel than an exported function type signature
changes.

I'm not sure how you know which exports are used only by in-tree modules
and which are used out of tree, but if you know that then you can version
them manually as we said by adding _v2 in the rare case you need to change
a behaviour.

So I'm still having trouble understanding what modversions is giving you.

> One thing that could work for us would be:
> 
> - Stricter version matching for in-tree modules (maybe some extra
>   part in vermagic that is skipped for out-of-tree modules)
> - Ability to blacklist use of a symbol, or all symbols in a module,
>   by out-of-tree modules
> 
> where the blacklist would be a matter of distribution policy.  But this
> would still require a fair amount of work by someone, and I doubt you'd
> want this upstream.

I don't think people are adverse to carrying some upstream complexity for
ditsros. Although for this fancy blacklist case, can it just be done in
userspace?

Thanks,
Nick

[toc] | [prev] | [next] | [standalone]

Page 2 of 5 — ← Prev page 1 [2] 3 4 5  Next page →

Back to top | Article view | linux.kernel

csiph-web