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

Page 2 of 2 — ← Prev page 1 [2]

#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]

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

On 2016-11-24 05:40, Ingo Molnar wrote:
> 
> * Adam Borowski <kilobyte@angband.pl> wrote:
> 
>> 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>
>> ---
>> Changes: corrected Peter Wu's address, added Tested-by: Oliver.
>> This is an unsplit version (x86/include/ and include/ together).
>>
>>  arch/x86/include/asm/asm-prototypes.h | 12 ++++++++++++
>>  include/asm-generic/asm-prototypes.h  |  7 +++++++
>>  2 files changed, 19 insertions(+)
>>  create mode 100644 arch/x86/include/asm/asm-prototypes.h
>>  create mode 100644 include/asm-generic/asm-prototypes.h
> 
> Michal, I'm quite unhappy about how the offending commit that broke modversions 
> for essentially _everyone_ who does more complex modular builds on x86 ended up 
> upstream:
> 
>   commit 4efca4ed05cbdfd13ec3e8cb623fb77d6e4ab187
>   Author:     Nicholas Piggin <npiggin@gmail.com>
>   AuthorDate: Tue Nov 1 12:46:19 2016 +1100
>   Commit:     Michal Marek <mmarek@suse.com>
>   CommitDate: Tue Nov 1 16:20:17 2016 +0100
> 
>       kbuild: modversions for EXPORT_SYMBOL() for asm
> 
>       Allow architectures to create asm/asm-prototypes.h file that
>       provides C prototypes for exported asm functions, which enables
>       proper CRC versions to be generated for them.
> 
>  scripts/Makefile.build | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 72 insertions(+), 6 deletions(-)
> 
> It was applied 4 hours after it was sent in the -rc3 timeframe, and then it went 
> upstream in -rc5:
> 
>      "Here are some regression fixes for kbuild:
>     
>        - modversion support for exported asm symbols (Nick Piggin). The
>          affected architectures need separate patches adding
>          asm-prototypes.h.
> 
> ... the fine merge log even says that the commit 'needs separate patches'!

It needs separate patches to finally fix the regression that happened in
-rc1.

Michal

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

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

(Adding Thorsten because this is a serious regression and still not fixed)

Adam Borowski <kilobyte@angband.pl> writes:

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

Calling it "may fail" is an understatement, for me _every_ 4.9-rc build
I have tried on my x86 32 bit test laptop has had broken module loading
and hence unable to boot. PLEASE fix this in Linus tree before he
releases 4.9, either applying this patch (which I currently apply
separately everytime I upgrade the kernel) or reverting the offending
commit which started the whole mess. It's not cool to keep the build
broken for this long, people have wasted lots of time because of this.

-- 
Kalle Valo

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

#1528848

Am 23.11.2016 um 21:53 schrieb Adam Borowski:
> Last version (rewritten description) is at:
> https://patchwork.kernel.org/patch/9439501/
> (needs s/oeter/peter/ for a typo in Peter Wu's address)

Hi Adam,

good to know. I kept track on it. For me it is easy to use patches.
However it would be also good having it in the final 4.9 release.

You can also add me as ack and tested:

Acked-by: Philip Mueller <philm@manjaro.org>
Tested-by: Philip Mueller <philm@manjaro.org>

Thx for the patch again.

Greez, Phil

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web