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

Quoted 16 lines · On Tue, 1 Nov 2016 13:48:59 +0100 Philip Müller <[email protected]> wrote:

Hi Nicholas, hi Michal,

due to following commit it seems the 64bit architecture of linux 4.9-rc is not able to boot at all, as it is unable to find its root device:

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=84d69848c97faab0c25aa2667b273404d2e2a64a

This got reported by several users:

https://forum.manjaro.org/t/linux-4-9-rc2-does-not-boot/11434

Even after applying that patch it seems still to be unbootable:

https://patchwork.kernel.org/patch/9406823/raw/

Quoted 7 lines

It is claimed it would boot by disabling CONFIG_MODVERSIONS for now. I still wonder why the 32bit version of the kernel boots without an issue.

Any thoughts/plans on the matter from your end? We are happy to test some patches if needed.

kind regards, Philip

Hi Philip,

Thanks for reporting. You likely need to apply the x86 specific patch as well as the above one.

http://marc.info/?l=linux-kernel&m=147669851906489&w=2

Thanks,
Nick

Hi Robert,

you have to apply following patch also:

provide-asm-prototypes.h-for-x86.patch:
https://patchwork.kernel.org/patch/9408985/raw/

@Adam, Nick: Was this patch not yet sent to Linus?

greez, Phil

Am 01.11.2016 um 14:46 schrieb Nicholas Piggin:

Quoted 38 lines

Quoted 16 lines · On Tue, 1 Nov 2016 13:48:59 +0100 Philip Müller <[email protected]> wrote:

Hi Nicholas, hi Michal,

due to following commit it seems the 64bit architecture of linux 4.9-rc is not able to boot at all, as it is unable to find its root device:

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=84d69848c97faab0c25aa2667b273404d2e2a64a

This got reported by several users:

https://forum.manjaro.org/t/linux-4-9-rc2-does-not-boot/11434

Even after applying that patch it seems still to be unbootable:

https://patchwork.kernel.org/patch/9406823/raw/

Quoted 7 lines

It is claimed it would boot by disabling CONFIG_MODVERSIONS for now. I still wonder why the 32bit version of the kernel boots without an issue.

Any thoughts/plans on the matter from your end? We are happy to test some patches if needed.

kind regards, Philip

Hi Philip,

Thanks for reporting. You likely need to apply the x86 specific patch as well as the above one.

http://marc.info/?l=linux-kernel&m=147669851906489&w=2

Thanks,
Nick

Quoted 2 lines

due to following commit it seems the 64bit architecture of linux 4.9-rc is not able to boot at all, as it is unable to find its root device:

you have to apply following patch also:

provide-asm-prototypes.h-for-x86.patch:
https://patchwork.kernel.org/patch/9408985/raw/

@Adam, Nick: Was this patch not yet sent to Linus?

Quoted 9 lines · Philip Müller

Quoted 2 lines

due to following commit it seems the 64bit architecture of linux 4.9-rc is not able to boot at all, as it is unable to find its root device:

you have to apply following patch also:

provide-asm-prototypes.h-for-x86.patch:
https://patchwork.kernel.org/patch/9408985/raw/

@Adam, Nick: Was this patch not yet sent to Linus?

The patch stewed in a kbuild-targetted thread since the morning after -rc1, Nick has recently requested that it should go through x86 maintainers instead. I've sent it there, lemme ping them, as the regression is severe and 4.9-final is close.

Apologies if I'm doing something wrong, I'm not a real kernel dev and merely was the person who came here looking for a fix, saw Nick's instructions and did the legwork implementing them.

Last version (rewritten description) is at:
https://patchwork.kernel.org/patch/9439501/
(needs s/oeter/peter/ for a typo in Peter Wu's address)

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?

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 <[email protected]>

Tested-by Kalle Valo <[email protected]>

Acked-by Nicholas Piggin <[email protected]>

Tested-by Peter Wu <[email protected]>

Tested-by Oliver Hartkopp <[email protected]>

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

Quoted 20 lines · Adam Borowski

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 <[email protected]>

Tested-by Kalle Valo <[email protected]>

Acked-by Nicholas Piggin <[email protected]>

Tested-by Peter Wu <[email protected]>

Tested-by Oliver Hartkopp <[email protected]>

---
Changes: corrected Peter Wu's address, added Tested-by: Oliver. This is an unsplit version (x86/include/ and include/ together).

Quoted 5 lines

 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 <[email protected]>
AuthorDate: Tue Nov 1 12:46:19 2016 +1100
Commit: Michal Marek <[email protected]>
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's still totally broken upstream and it didn't fix any regressions AFAICS (or if it did then its changelog was very silent on that fact).

Why was such a complex patch applied and why isn't it reverted or fixed upstream?

Quoted 8 lines

 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:

Quoted 8 lines

- 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's still totally broken upstream and it didn't fix any regressions AFAICS (or if it did then its changelog was very silent on that fact).

Well it doesn't fix regression by itself, as discussed it needs architecture patches. I've tried keeping linux-arch on cc for all this modversion breakage stuff since it became clear it would require arch changes.

The actual x86 bug I suppose you would say is caused by 784d5699eddc5. But I should probably have included more background in the above initial crc support patch, e.g, at least reference 22823ab419d. So mea culpa for that.

Quoted 17 lines · Nicholas Piggin

Quoted 8 lines

 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:

Quoted 8 lines

- 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's still totally broken upstream and it didn't fix any regressions AFAICS (or if it did then its changelog was very silent on that fact).

Quoted 7 lines

Well it doesn't fix regression by itself, as discussed it needs architecture patches. I've tried keeping linux-arch on cc for all this modversion breakage stuff since it became clear it would require arch changes.

The actual x86 bug I suppose you would say is caused by 784d5699eddc5. But I should probably have included more background in the above initial crc support patch, e.g, at least reference 22823ab419d. So mea culpa for that.

Indeed 784d5699eddc5 makes more sense:

784d5699eddc ("x86: move exports to actual definitions") 22823ab419d8 ("EXPORT_SYMBOL() for asm")

... and sorry about coming down on you and Marek!

I've Cc:-ed Al.

I think what happened is that 22823ab419d8 and 784d5699eddc caused the boot regression (modular builds with modversions enabled not booting), and your fix half-fixed it - with the remaining fix (that adds the header to x86) fixing the rest.

Still the fact remains that modversions was broken in -rc1 which delayed testing done by a number of prominent testers. :-(

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?

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)

Quoted 6 lines · Nicholas Piggin

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.

Quoted 2 lines

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?

Quoted 5 lines

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/

Quoted 10 lines · On Thu, 24 Nov 2016 08:36:39 +0100 Greg Kroah-Hartman…

Quoted 8 lines · Nicholas Piggin

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?

Quoted 2 lines

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.

Quoted 13 lines · Nicholas Piggin

Quoted 10 lines · On Thu, 24 Nov 2016 08:36:39 +0100 Greg Kroah-Hartman…

Quoted 8 lines · Nicholas Piggin

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?

Quoted 6 lines

Quoted 2 lines

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.

The problem is that with every kernel release, the structures change in a way that you would have to bump the version of virtually every export.

At which point, there would be little difference between CONFIG_MODVERSION on and off (without CONFIG_MODVERSION, we compare the kernel version strings when loading modules).

Quoted 24 lines · On Thu, 24 Nov 2016 10:32:12 +0100 Michal Marek <[email protected]> wrote:

Quoted 13 lines · Nicholas Piggin

Quoted 10 lines · On Thu, 24 Nov 2016 08:36:39 +0100 Greg Kroah-Hartman…

Quoted 8 lines · Nicholas Piggin

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?

Quoted 6 lines

Quoted 2 lines

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.

The problem is that with every kernel release, the structures change in a way that you would have to bump the version of virtually every export.

Quoted 3 lines

At which point, there would be little difference between CONFIG_MODVERSION on and off (without CONFIG_MODVERSION, we compare the kernel version strings when loading modules).

I'm not sure about that. If they are truly incompatible changes and MODVERSIONS does not pick up a different CRC, then it's even worse if incompatible modules are missed so often.

Quoted 11 lines

Quoted 5 lines

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?

Quoted 2 lines

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.

Quoted 14 lines · CET Nicholas Piggin

Quoted 13 lines

Quoted 5 lines

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/

Quoted 5 lines

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.

It's certainly not an option for v4.9 at this point. There is also no

realistic way we can get a correct asm/asm-prototypes.h for all the other architectures in place. At the moment, powerpc is the only one that actually works with modversions.

We can either make CONFIG_MODVERSIONS a per-architecture opt-in and let only the ones that have the header file select that, or revert all of Al's original patches that moved the exports.

Quoted 7 lines · On Thu, 24 Nov 2016 08:36:39 +0100 Greg Kroah-Hartman…

Quoted 6 lines · Nicholas Piggin

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.

Quoted 5 lines

Quoted 2 lines

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.

Quoted 10 lines

Quoted 4 lines

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?

Quoted 2 lines

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.

Quoted 20 lines · Nicholas Piggin

Quoted 7 lines · On Thu, 24 Nov 2016 08:36:39 +0100 Greg Kroah-Hartman…

Quoted 6 lines · Nicholas Piggin

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.

Quoted 5 lines

Quoted 2 lines

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.

Quoted 3 lines

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.

You need a tool that looks
at either the source code (what we have today), or looks at the

object/debugging code (like the link I pointed at.)

Quoted 10 lines

Quoted 10 lines

Quoted 4 lines

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?

Quoted 8 lines

Quoted 2 lines

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.

But more importantly, that's a much longer-term solution, fixing what we have today to at least start working again is much more important before we start bikeshedding the whole mess :)

Quoted 25 lines · On Thu, 24 Nov 2016 10:56:22 +0100 Greg Kroah-Hartman…

Quoted 23 lines · Nicholas Piggin

Quoted 7 lines · On Thu, 24 Nov 2016 08:36:39 +0100 Greg Kroah-Hartman…

Quoted 6 lines · Nicholas Piggin

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.

Quoted 5 lines

Quoted 2 lines

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.

Quoted 2 lines

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.

Quoted 2 lines

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

Does not follow down all possible derivable pointer types.

Quoted 19 lines

Quoted 10 lines

Quoted 10 lines

Quoted 4 lines

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?

Quoted 8 lines

Quoted 2 lines

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.

Quoted 4 lines

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?

Quoted 28 lines · Nicholas Piggin

Quoted 25 lines · On Thu, 24 Nov 2016 10:56:22 +0100 Greg Kroah-Hartman…

Quoted 23 lines · Nicholas Piggin

Quoted 7 lines · On Thu, 24 Nov 2016 08:36:39 +0100 Greg Kroah-Hartman…

Quoted 6 lines · Nicholas Piggin

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.

Quoted 5 lines

Quoted 2 lines

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.

Quoted 6 lines

Quoted 2 lines

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.

Quoted 5 lines

Quoted 2 lines

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.

Quoted 1 line

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.

Quoted 24 lines

Quoted 19 lines

Quoted 10 lines

Quoted 10 lines

Quoted 4 lines

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?

Quoted 8 lines

Quoted 2 lines

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.

Quoted 4 lines

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.

Quoted 8 lines

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

Quoted 2 lines

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?

Quoted 6 lines · Nicholas Piggin

Quoted 2 lines

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.

Quoted 4 lines

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

Quoted 6 lines · Nicholas Piggin

Quoted 2 lines

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.

Quoted 3 lines

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.

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

Quoted 7 lines · Nicholas Piggin

Quoted 6 lines

Quoted 2 lines

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.

Quoted 2 lines

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:

Quoted 16 lines

Quoted 4 lines

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.

Quoted 2 lines · On Tue, 29 Nov 2016 01:15:48 +0000 Ben Hutchings <[email protected]> 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.

Quoted 18 lines · Linus Torvalds

Quoted 7 lines · Nicholas Piggin

Quoted 6 lines

Quoted 2 lines

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.

Quoted 2 lines

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:

Review

Quoted 3 lines

(quoted diff) +2 -1

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

Quoted 19 lines

Quoted 10 lines

Quoted 4 lines

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.

Quoted 6 lines

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:

Quoted 6 lines

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.

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.

Quoted 4 lines

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:

Quoted 7 lines · Ben Hutchings

Quoted 4 lines

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

So somebody send me a minimal patch that is

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

and I'll happily re-enable modversions.

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/[email protected]/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.

Quoted 4 lines · Adam Borowski

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

Quoted 9 lines

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.

Quoted 2 lines

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.

So somebody send me a minimal patch that is

(a) tested

(b) explains it

(c) obvious

and I'll happily re-enable modversions.

Quoted 4 lines

(a) tested

By many people.

Quoted 1 line

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

Quoted 6 lines · On Nov 29, 2016 5:51 AM, "Adam Borowski" <[email protected]…

Quoted 4 lines

(a) tested

By many people.

No.

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

Quoted 2 lines

Quoted 1 line

(b) explains it

Quoted 3 lines

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.

Quoted 12 lines · Michal Marek

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

Quoted 4 lines · Ben Hutchings

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.

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

In its place I just added a simple config option to override vermagic so distros can manage it entirely themselves.

Quoted 5 lines · Nicholas Piggin

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.

Quoted 2 lines

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.

Quoted 11 lines · Linus Torvalds

Quoted 5 lines · Nicholas Piggin

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.

Quoted 5 lines

Quoted 2 lines

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.

Quoted 4 lines

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.

On Wed, 30 Nov 2016 21:33:01 +0000

Quoted 30 lines

Ben Hutchings <[email protected]> wrote:

Quoted 11 lines · Linus Torvalds

Quoted 5 lines · Nicholas Piggin

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.

Quoted 5 lines

Quoted 2 lines

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.

Quoted 4 lines

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

Quoted 5 lines

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.

Quoted 7 lines

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

Quoted 3 lines

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?

Quoted 37 lines · Nicholas Piggin

On Wed, 30 Nov 2016 21:33:01 +0000

Quoted 30 lines

Ben Hutchings <[email protected]> wrote:

Quoted 11 lines · Linus Torvalds

Quoted 5 lines · Nicholas Piggin

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.

Quoted 5 lines

Quoted 2 lines

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.

Quoted 4 lines

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

Quoted 5 lines

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.

Quoted 5 lines

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.

As I understand it, genksyms incorporates the definitions of a function's parameter and return types - not just their names - and all the types they refer to, recursively. So a structure size change should change the version of all functions where the function and its caller pass that structure between them, however indirectly. It finds such indirect ABI breakage for me fairly regularly, though of course I don't know that it finds everything.

Quoted 4 lines

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.

That's fine for individual functions.

Quoted 1 line

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

Where there is a family of driver modules (e.g. foo-core, foo-pci, foo- usb), a structure change can change all exports from foo-core. That ABI is of no use to out-of-tree drivers so we don't care about keeping it stable, but we do care about preventing an accidental mismatch.

Quoted 12 lines

Quoted 11 lines

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.

Quoted 3 lines

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?

Since the kernel does the symbol lookup and version matching, I'm not sure what userland can do about it.

Quoted 7 lines · On Thu, 01 Dec 2016 02:35:54 +0000 Ben Hutchings <[email protected]> wrote:

As I understand it, genksyms incorporates the definitions of a function's parameter and return types - not just their names - and all the types they refer to, recursively. So a structure size change should change the version of all functions where the function and its caller pass that structure between them, however indirectly. It finds such indirect ABI breakage for me fairly regularly, though of course I don't know that it finds everything.

It is only the type name.

Not only that but even if you did extend it further to structure type arrangement then you still have to deal with other structures followed via pointers. Or (rarer but not unheard of):

- changes to structures without changes of the types of their members
- changes to arguments without changes of their type

- changes to semantics of functions
- data structures derived in ways other than exported symbols, e.g., fixed register for current on some archs

Quoted 16 lines · Nicholas Piggin

Quoted 7 lines · On Thu, 01 Dec 2016 02:35:54 +0000 Ben Hutchings <[email protected]> wrote:

As I understand it, genksyms incorporates the definitions of a function's parameter and return types - not just their names - and all the types they refer to, recursively. So a structure size change should change the version of all functions where the function and its caller pass that structure between them, however indirectly. It finds such indirect ABI breakage for me fairly regularly, though of course I don't know that it finds everything.

It is only the type name.

Not only that but even if you did extend it further to structure type arrangement then you still have to deal with other structures followed via pointers. Or (rarer but not unheard of):

Quoted 2 lines

- changes to structures without changes of the types of their members
- changes to arguments without changes of their type

This is already covered by genksyms. Try make V=1 with CONFIG_MODVERSIONS=y and add the -D option to one of the genksyms command. I wanted to paste the expanded signature for register_filesystem() as an example, but vger would probably drop the mail for being too big :).

Quoted 16 lines · Nicholas Piggin

Quoted 7 lines · On Thu, 01 Dec 2016 02:35:54 +0000 Ben Hutchings <[email protected]> wrote:

As I understand it, genksyms incorporates the definitions of a function's parameter and return types - not just their names - and all the types they refer to, recursively. So a structure size change should change the version of all functions where the function and its caller pass that structure between them, however indirectly. It finds such indirect ABI breakage for me fairly regularly, though of course I don't know that it finds everything.

It is only the type name.

Not only that but even if you did extend it further to structure type arrangement then you still have to deal with other structures followed via pointers. Or (rarer but not unheard of):

Quoted 2 lines

- changes to structures without changes of the types of their members
- changes to arguments without changes of their type

This is already covered by genksyms. Try make V=1 with CONFIG_MODVERSIONS=y and add the -D option to one of the genksyms command. I wanted to paste the expanded signature for register_filesystem() as an example, but vger would probably drop the mail for being too big :).

Well I simply tested the outcome. If you have:

struct blah {
  int x;
};
int foo(struct blah *blah)
{
  return blah->x;
}
EXPORT(foo);

$ nm vmlinux | grep __crc_foo
00000000a0cf13a0 A __crc_foo

Now change to

struct blah {
  int y;
  int x;
};

$ nm vmlinux | grep __crc_foo
00000000a0cf13a0 A __crc_foo

It just doesn't catch these things.

Quoted 22 lines · Nicholas Piggin

Well I simply tested the outcome. If you have:

struct blah {
  int x;
};
int foo(struct blah *blah)
{
  return blah->x;
}
EXPORT(foo);

$ nm vmlinux | grep __crc_foo
00000000a0cf13a0 A __crc_foo

Now change to

struct blah {
  int y;
  int x;
};

Quoted 4 lines

$ nm vmlinux | grep __crc_foo
00000000a0cf13a0 A __crc_foo

It just doesn't catch these things.

I found the same when I just added your snippet to init/main.c.

_But_ when I moved the struct into include/types.h (which happened to be included by init/main.c) then, with just x in the struct:

    $ make -s init/main.{o,symtypes} && grep -E foo\|blah init/main.symtypes && nm init/main.o  | grep __crc_foo
    s#blah struct blah { int x ; } 
    foo int foo ( s#blah * ) 
    000000000cd0312e A __crc_foo

but adding y:

    $ make -s init/main.{o,symtypes} && grep -E foo\|blah init/main.symtypes && nm init/main.o  | grep __crc_foo
    s#blah struct blah { int x ; int y ; } 
    foo int foo ( s#blah * ) 
    00000000eda220c6 A __crc_foo

So it does catch things in that case.

With struct blah inline in main.c it was:

    $ make -s init/main.{o,symtypes} && grep -E foo\|blah init/main.symtypes && nm init/main.o  | grep __crc_foo
    s#blah struct blah { UNKNOWN } 
    foo int foo ( s#blah * ) 
    00000000a0cf13a0 A __crc_foo

So I suppose it only cares about structs which are in headers, which I guess makes sense. I think it is working in at least one of the important cases.

That said, a dwarf based checker tool should be able to do as good a job (maybe a bit better because report is very informative and it may pick up compiler alignments or padding options).

Hello,

Nicholas Piggin <[email protected]> a écrit:

[...]

Quoted 3 lines

That said, a dwarf based checker tool should be able to do as good a job (maybe a bit better because report is very informative and it may pick up compiler alignments or padding options).

So, Nicholas was kind enough to send me the two Linux Kernel binaries that he built with the tiny little interface change that we were discussing earlier. Here is what the abidiff[1] tools says about that interface change:

    $ time ~/git/libabigail/kabidiff/build/tools/abidiff vmlinux.abi1.abi vmlinux.abi2.abi
    Functions changes summary: 0 Removed, 1 Changed, 0 Added function
    Variables changes summary: 0 Removed, 0 Changed, 0 Added variable

1 function with some indirect sub-type change:

[C]'function int foo(blah*)' at memory.c:82:1 has some indirect sub-type changes:
parameter 1 of type 'blah*' has sub-type changes:
in pointed to type 'struct blah' at memory.c:78:1:
type size changed from 32 to 64 bits
1 data member insertion:
'int blah::y', at offset 0 (in bits) at memory.c:79:1
1 data member change:
'int blah::x' offset changed from 0 to 32 (in bits) (by +32 bits)

real 0m2.595s
user 0m2.489s
sys 0m0.108s
$

I kept the timing information to give you an idea of the time it takes on a non-optimized build of abidiff.

One could for instance want that types that are not defined in header files be kept out of the change report. In that case it's possible to write a little suppression specification file like this one:

    $ cat vmlinux.abignore 
    [suppress_type]
      source_location_not_regexp = .*\\.h
    $

You can then pass that suppression file to the tool:

    $ ~/git/libabigail/kabidiff/build/tools/abidiff --suppr vmlinux.abignore vmlinux.abi1.abi vmlinux.abi2.abi
    Functions changes summary: 0 Removed, 0 Changed (1 filtered out), 0 Added function
    Variables changes summary: 0 Removed, 0 Changed, 0 Added variable

    real	0m2.574s
    user	0m2.473s
    sys	0m0.102s
    $

So this is the kind of interface change analysis tool we are working on at the moment.

One could also imagine a tool that would compute a CRC that takes the very same suppression specification files into account, letting people to decide that some interface changes are OK. That CRC would thus be added to the special ELF sections we already have today. We could keep the modversion machinery, but with a greater dose of flexibility. Whenever modversion detects a change, abidiff would tell people what the change is exactly.

What do you guys think?

Quoted 72 lines · Dodji Seketeli

Hello,

Nicholas Piggin <[email protected]> a écrit:

[...]

Quoted 3 lines

That said, a dwarf based checker tool should be able to do as good a job (maybe a bit better because report is very informative and it may pick up compiler alignments or padding options).

So, Nicholas was kind enough to send me the two Linux Kernel binaries that he built with the tiny little interface change that we were discussing earlier. Here is what the abidiff[1] tools says about that interface change:

    $ time ~/git/libabigail/kabidiff/build/tools/abidiff vmlinux.abi1.abi vmlinux.abi2.abi
    Functions changes summary: 0 Removed, 1 Changed, 0 Added function
    Variables changes summary: 0 Removed, 0 Changed, 0 Added variable

1 function with some indirect sub-type change:

[C]'function int foo(blah*)' at memory.c:82:1 has some indirect sub-type changes:
parameter 1 of type 'blah*' has sub-type changes:
in pointed to type 'struct blah' at memory.c:78:1:
type size changed from 32 to 64 bits
1 data member insertion:
'int blah::y', at offset 0 (in bits) at memory.c:79:1
1 data member change:
'int blah::x' offset changed from 0 to 32 (in bits) (by +32 bits)

real 0m2.595s
user 0m2.489s
sys 0m0.108s
$

I kept the timing information to give you an idea of the time it takes on a non-optimized build of abidiff.

One could for instance want that types that are not defined in header files be kept out of the change report. In that case it's possible to write a little suppression specification file like this one:

    $ cat vmlinux.abignore 
    [suppress_type]
      source_location_not_regexp = .*\\.h
    $

You can then pass that suppression file to the tool:

    $ ~/git/libabigail/kabidiff/build/tools/abidiff --suppr vmlinux.abignore vmlinux.abi1.abi vmlinux.abi2.abi
    Functions changes summary: 0 Removed, 0 Changed (1 filtered out), 0 Added function
    Variables changes summary: 0 Removed, 0 Changed, 0 Added variable

    real	0m2.574s
    user	0m2.473s
    sys	0m0.102s
    $

So this is the kind of interface change analysis tool we are working on at the moment.

One could also imagine a tool that would compute a CRC that takes the very same suppression specification files into account, letting people to decide that some interface changes are OK. That CRC would thus be added to the special ELF sections we already have today. We could keep the modversion machinery, but with a greater dose of flexibility. Whenever modversion detects a change, abidiff would tell people what the change is exactly.

Quoted 1 line

What do you guys think?

YES YES YES!!!

Now I don't work on a distro anymore, but I would think that something like this would be really useful, pointing out exactly what changed is very important for distro maintainers to determine what they want to do (either fix up the abi change with strange hacks, or ignore it due to the change being in an area they don't care at all about, i.e. a random driver subsystem.)

So yes, I think this is really good stuff. But if the distro maintainers correct me and think it's useless, then I need to revisit my view of exactly what they do for their customers :)

Now I don't work on a distro anymore, but I would think that something like this would be really useful, pointing out exactly what changed is very important for distro maintainers to determine what they want to do

Quoted 72 lines · Dodji Seketeli

Hello,

Nicholas Piggin <[email protected]> a �crit:

[...]

Quoted 3 lines

That said, a dwarf based checker tool should be able to do as good a job (maybe a bit better because report is very informative and it may pick up compiler alignments or padding options).

So, Nicholas was kind enough to send me the two Linux Kernel binaries that he built with the tiny little interface change that we were discussing earlier. Here is what the abidiff[1] tools says about that interface change:

    $ time ~/git/libabigail/kabidiff/build/tools/abidiff vmlinux.abi1.abi vmlinux.abi2.abi
    Functions changes summary: 0 Removed, 1 Changed, 0 Added function
    Variables changes summary: 0 Removed, 0 Changed, 0 Added variable

1 function with some indirect sub-type change:

[C]'function int foo(blah*)' at memory.c:82:1 has some indirect sub-type changes:
parameter 1 of type 'blah*' has sub-type changes:
in pointed to type 'struct blah' at memory.c:78:1:
type size changed from 32 to 64 bits
1 data member insertion:
'int blah::y', at offset 0 (in bits) at memory.c:79:1
1 data member change:
'int blah::x' offset changed from 0 to 32 (in bits) (by +32 bits)

real 0m2.595s
user 0m2.489s
sys 0m0.108s
$

I kept the timing information to give you an idea of the time it takes on a non-optimized build of abidiff.

One could for instance want that types that are not defined in header files be kept out of the change report. In that case it's possible to write a little suppression specification file like this one:

    $ cat vmlinux.abignore 
    [suppress_type]
      source_location_not_regexp = .*\\.h
    $

You can then pass that suppression file to the tool:

    $ ~/git/libabigail/kabidiff/build/tools/abidiff --suppr vmlinux.abignore vmlinux.abi1.abi vmlinux.abi2.abi
    Functions changes summary: 0 Removed, 0 Changed (1 filtered out), 0 Added function
    Variables changes summary: 0 Removed, 0 Changed, 0 Added variable

    real	0m2.574s
    user	0m2.473s
    sys	0m0.102s
    $

So this is the kind of interface change analysis tool we are working on at the moment.

One could also imagine a tool that would compute a CRC that takes the very same suppression specification files into account, letting people to decide that some interface changes are OK. That CRC would thus be added to the special ELF sections we already have today. We could keep the modversion machinery, but with a greater dose of flexibility. Whenever modversion detects a change, abidiff would tell people what the change is exactly.

Quoted 1 line

What do you guys think?

YES YES YES!!!

Now I don't work on a distro anymore, but I would think that something like this would be really useful, pointing out exactly what changed is very important for distro maintainers to determine what they want to do (either fix up the abi change with strange hacks, or ignore it due to the change being in an area they don't care at all about, i.e. a random driver subsystem.)

So yes, I think this is really good stuff. But if the distro maintainers correct me and think it's useless, then I need to revisit my view of exactly what they do for their customers :)

Quoted 3 lines

That said, a dwarf based checker tool should be able to do as good a job (maybe a bit better because report is very informative and it may pick up compiler alignments or padding options).

So, Nicholas was kind enough to send me the two Linux Kernel binaries that he built with the tiny little interface change that we were discussing earlier. Here is what the abidiff[1] tools says about that interface change:

Hello,

Nicholas Piggin <[email protected]> a écrit:

[...]

Quoted 3 lines

That said, a dwarf based checker tool should be able to do as good a job (maybe a bit better because report is very informative and it may pick up compiler alignments or padding options).

So, Nicholas was kind enough to send me the two Linux Kernel binaries that he built with the tiny little interface change that we were discussing earlier. Here is what the abidiff[1] tools says about that interface change:

    $ time ~/git/libabigail/kabidiff/build/tools/abidiff vmlinux.abi1.abi vmlinux.abi2.abi
    Functions changes summary: 0 Removed, 1 Changed, 0 Added function
    Variables changes summary: 0 Removed, 0 Changed, 0 Added variable

1 function with some indirect sub-type change:

[C]'function int foo(blah*)' at memory.c:82:1 has some indirect sub-type changes:
parameter 1 of type 'blah*' has sub-type changes:
in pointed to type 'struct blah' at memory.c:78:1:
type size changed from 32 to 64 bits
1 data member insertion:
'int blah::y', at offset 0 (in bits) at memory.c:79:1
1 data member change:
'int blah::x' offset changed from 0 to 32 (in bits) (by +32 bits)

Does the abidiff tool handle the case when an exported symbol is moved between .c files? This is always a mess with genksyms, because the two .c files have different includes and thus the type expansion stops at different points. So typically the move needs to be reverted as a workaround.

Michal Marek <[email protected]> a écrit:

[...]

Quoted 5 lines

Does the abidiff tool handle the case when an exported symbol is moved between .c files? This is always a mess with genksyms, because the two .c files have different includes and thus the type expansion stops at different points. So typically the move needs to be reverted as a workaround.

Let's consider the function:

'void foo(struct S*);'

If two ELF binaries contain a definition of that function foo which ELF symbol is exported, if the type struct S hasn't changed, and if the only difference between the ELF binaries is that foo was defined in the translation unit a.c in the first binary and in b.c in the second binary, then the comparison engine of libabigail (which is the library that abidiff uses) will consider the declarations of the two foo functions as being equal -- no matter what include file comes before the definition point of foo in a.c and b.c. If it does not, then it's a bug that ought to be fixed.

If you feel that I haven't understood your question, then I guess a minimal standalone example (in the form of C source code) that illustrates your use case could be helpful to me.

A minimal example would be

t1.c:
struct s1;

struct s2 {
	int i;
}
struct s3 {
	struct s1 *ptr1;
	struct s2 *ptr2;
}

void foo(struct s3*);
EXPORT_SYMBOL(foo);

t2.c:

struct s1 {
	int j;
}
struct s2;
struct s3 {
	struct s1 *ptr1;
	struct s2 *ptr2;
}

void foo(struct s3*);
EXPORT_SYMBOL(foo);

genksyms expands this to
void foo ( struct s3 { struct s1 { UNKNOWN } * ptr1 ; struct s2 { int i ; } * ptr2 ; } * )

or

void foo ( struct s3 { struct s1 { int j ; } * ptr1 ; struct s2 { UNKNOWN } * ptr2 ; } * ) respectively.

The types are the same, but their visibility in the different compilation units differs.

Michal Marek <[email protected]> a écrit:

[...]

Quoted 26 lines

A minimal example would be

t1.c:
struct s1;

struct s2 {
	int i;
}
struct s3 {
	struct s1 *ptr1;
	struct s2 *ptr2;
}

void foo(struct s3*);
EXPORT_SYMBOL(foo);

t2.c:

struct s1 {
	int j;
}
struct s2;
struct s3 {
	struct s1 *ptr1;
	struct s2 *ptr2;
}

void foo(struct s3*);
EXPORT_SYMBOL(foo);

Quoted 7 lines

genksyms expands this to
void foo ( struct s3 { struct s1 { UNKNOWN } * ptr1 ; struct s2 { int i ; } * ptr2 ; } * )

or

void foo ( struct s3 { struct s1 { int j ; } * ptr1 ; struct s2 { UNKNOWN } * ptr2 ; } * ) respectively.

Thanks, I have built an independant test case from this:

$ cat t1.c
struct s1;
struct s2 {
	int i;
};
struct s3 {
	struct s1 *ptr1;
	struct s2 *ptr2;
};
void foo(struct s3*);
$ cat t2.c
struct s1 {
	int j;
};
struct s2;
struct s3 {
	struct s1 *ptr1;
	struct s2 *ptr2;
};
void foo(struct s3*);
$ gcc -g -c t1.c
$ gcc -g -c t2.c
$ abidiff t1.o t2.o
$ 

So, as you see here, abidiff considers t1.o and t2.o has having the same ABI, so it considers the two foo functions to be equivalent.

Quoted 2 lines

The types are the same, but their visibility in the different compilation units differs.

I see, for genksyms, the order of declarations matters, especially when forward declarations are involved.

Libabigail does a "whole binary" analysis of types.

So, consider the point of use of the type 'struct s1*'. Even if 'struct s' is just forward-declared at that point, the declaration of struct s1 is "resolved" to its definition. Even if the definition comes later in the binary.

Quoted 6 lines

Libabigail does a "whole binary" analysis of types.

So, consider the point of use of the type 'struct s1*'. Even if 'struct s' is just forward-declared at that point, the declaration of struct s1 is "resolved" to its definition. Even if the definition comes later in the binary.

But there isn't any definition of struct s1 in t1.o. Does abidiff "steal" the definition from the other object file? That would be legitimate, I'm just curious.

Quoted 10 lines · Michal Marek

Quoted 6 lines

Libabigail does a "whole binary" analysis of types.

So, consider the point of use of the type 'struct s1*'. Even if 'struct s' is just forward-declared at that point, the declaration of struct s1 is "resolved" to its definition. Even if the definition comes later in the binary.

But there isn't any definition of struct s1 in t1.o. Does abidiff "steal" the definition from the other object file? That would be legitimate, I'm just curious.

If there is another translation unit in the *same* binary that defines struct s1, then yes, it's "stolen", as you say.

But if in the entire binary, struct s1 is just declared (not defined), then it'll compare equal to any struct s1 that is defined in the *second* binary.

So, as you see here, abidiff considers t1.o and t2.o has having the same ABI, so it considers the two foo functions to be equivalent.

Quoted 2 lines

The types are the same, but their visibility in the different compilation units differs.

I see, for genksyms, the order of declarations matters, especially when forward declarations are involved.

Libabigail does a "whole binary" analysis of types.

So, consider the point of use of the type 'struct s1*'. Even if 'struct s' is just forward-declared at that point, the declaration of struct s1 is "resolved" to its definition. Even if the definition comes later in the binary.

In other words, if struct s1 is defined in the binary, you'll never have that "struct s1 {UNKNOWN} *ptr1;" that you see in genksyms's representation.

A minimal example would be

t1.c:
struct s1;

struct s2 {
	int i;
}
struct s3 {
	struct s1 *ptr1;
	struct s2 *ptr2;
}

void foo(struct s3*);
EXPORT_SYMBOL(foo);

t2.c:

struct s1 {
	int j;
}
struct s2;
struct s3 {
	struct s1 *ptr1;
	struct s2 *ptr2;
}

void foo(struct s3*);
EXPORT_SYMBOL(foo);

Quoted 4 lines · Nicholas Piggin

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.

Quoted 2 lines

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

Quoted 11 lines · Linus Torvalds

Quoted 4 lines · Nicholas Piggin

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.

Quoted 8 lines

Quoted 2 lines

In its place I just added a simple config option to override vermagic so distros can manage it entirely themselves.

So at least Fedora doesn't even enable CONFIG_MODVERSIONS as-is. I'm _hoping_ it's just Debian that wants this, and we'd need to get some input from the Debian people whether that "control vermagic" is sufficient? I suspect it isn't, but I can't come up with any simple alternate model either..

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

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

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

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

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

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

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

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

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

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

Quoted 3 lines

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

Okay. It would be good to get all the distros in on this.

What I want to do is work out exactly what it is that modversions is giving you.

We know it's fairly nasty code to maintain and it does not detect ABI changes very well. But it's not such a burden that we can't maintain it if there are good reasons to keep it.

Quoted 4 lines

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

Basically vermagic is the string all modules and the kernel get, which must match in order to load modules. If you have modversions disabled, then vermagic includes the kernel version. If modversions is enabled, then vermagic does not include the kernel version but the CRCs have to also match.

Controlling it explicitly is just a couple of lines where a distro can control it (so they can update their kernel version without breaking). It's not meant to solve everything, just the first one.

Quoted 8 lines

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

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

Quoted 7 lines

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

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

I don't think that's necessary. A feature requirement for a distro is just as valid as any other user of upstream. I don't want to hinder any distro, I'm just still not quite seeing the big picture of exactly what functionality you need from the kernel.

Thanks,
Nick

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

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

Quoted 8 lines · Don Zickus

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

Quoted 2 lines

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

All that is counter to what we really want to have: a well integrated kernel that moves forward together so that we can see and improve the whole situation. No need to make things worse just to help leeches. Get your damn drivers upstream ASAP and let's stop this discussion..

Unfortunately, there are various drivers that will never go upstream

- paid storage drivers that provide bells and whistles on top of inbox driver

- old drivers/fs that application has been relying on for a long time but company doesn't have resources to migrate to current technology.

Quoted 4 lines · Don Zickus

Unfortunately, there are various drivers that will never go upstream

- paid storage drivers that provide bells and whistles on top of inbox driver

That's because the developer doesn't want them upstream, that's their fault, nothing we can do about them.

Quoted 2 lines

- old drivers/fs that application has been relying on for a long time but company doesn't have resources to migrate to current technology.

That's what drivers/staging/ is for, I'll take anything that builds (and sometimes stuff that doesn't build) as long as people are actually using it. So send the stuff that is in this category on to me and that will reduce your burden a _lot_.

Quoted 9 lines · Nicholas Piggin

Quoted 3 lines

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

Okay. It would be good to get all the distros in on this.

What I want to do is work out exactly what it is that modversions is giving you.

Quoted 3 lines

We know it's fairly nasty code to maintain and it does not detect ABI changes very well. But it's not such a burden that we can't maintain it if there are good reasons to keep it.

Hi Nick,

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

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

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

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

Scary, huh? :-)

Quoted 1 line

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

As a tool to aid distro reviewers, modversions has some value, but the debug info parsing tools that have been mentioned in this thread seem superior (not that I've tested them).

Quoted 4 lines

Quoted 1 line

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

As a tool to aid distro reviewers, modversions has some value, but the debug info parsing tools that have been mentioned in this thread seem superior (not that I've tested them).

On the other hand the big advantage of modversions is that it also verifies the checksum during runtime (module loading). In other words, I believe that any other solution should still generate some form of checksum/watermark which can be easily checked for compatibility on module load.
It should not be hard to add to the DWARF based tools though. We'd just parse DWARF data instead of the C code.

Quoted 11 lines

Quoted 4 lines

Quoted 1 line

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

As a tool to aid distro reviewers, modversions has some value, but the debug info parsing tools that have been mentioned in this thread seem superior (not that I've tested them).

On the other hand the big advantage of modversions is that it also verifies the checksum during runtime (module loading). In other words, I believe that any other solution should still generate some form of checksum/watermark which can be easily checked for compatibility on module load.
It should not be hard to add to the DWARF based tools though. We'd just parse DWARF data instead of the C code.

A runtime check is still done, with per-module vermagic which distros can change when they bump the ABI version. Is it really necessary to have more than that (i.e., per-symbol versioning)?

Quoted 11 lines

Quoted 11 lines

Quoted 4 lines

Quoted 1 line

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

As a tool to aid distro reviewers, modversions has some value, but the debug info parsing tools that have been mentioned in this thread seem superior (not that I've tested them).

On the other hand the big advantage of modversions is that it also verifies the checksum during runtime (module loading). In other words, I believe that any other solution should still generate some form of checksum/watermark which can be easily checked for compatibility on module load.
It should not be hard to add to the DWARF based tools though. We'd just parse DWARF data instead of the C code.

Quoted 3 lines

A runtime check is still done, with per-module vermagic which distros can change when they bump the ABI version. Is it really necessary to have more than that (i.e., per-symbol versioning)?

From my point of view, it is. We need to allow changing ABI for some
modules while maintaining it for others.
In fact I think that there should be version not only for every exported symbol (in the EXPORT_SYMBOL() sense), but also for every public type (in the sense of eg. structure defined in the public header file).

Quoted 15 lines · On Fri, 9 Dec 2016 15:36:04 +0100 Stanislav Kozina <[email protected]> wrote:

Quoted 14 lines

Quoted 11 lines

Quoted 4 lines

Quoted 1 line

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

As a tool to aid distro reviewers, modversions has some value, but the debug info parsing tools that have been mentioned in this thread seem superior (not that I've tested them).

On the other hand the big advantage of modversions is that it also verifies the checksum during runtime (module loading). In other words, I believe that any other solution should still generate some form of checksum/watermark which can be easily checked for compatibility on module load.
It should not be hard to add to the DWARF based tools though. We'd just parse DWARF data instead of the C code.

A runtime check is still done, with per-module vermagic which distros can change when they bump the ABI version. Is it really necessary to have more than that (i.e., per-symbol versioning)?

Quoted 5 lines

From my point of view, it is. We need to allow changing ABI for some
modules while maintaining it for others.
In fact I think that there should be version not only for every exported symbol (in the EXPORT_SYMBOL() sense), but also for every public type (in the sense of eg. structure defined in the public header file).

Well the distro can just append _v2, _v3 to the name of the function or type if it has to break compat for some reason. Would that be enough?

Quoted 8 lines

Quoted 8 lines

Quoted 3 lines

A runtime check is still done, with per-module vermagic which distros can change when they bump the ABI version. Is it really necessary to have more than that (i.e., per-symbol versioning)?

From my point of view, it is. We need to allow changing ABI for some
modules while maintaining it for others.
In fact I think that there should be version not only for every exported symbol (in the EXPORT_SYMBOL() sense), but also for every public type (in the sense of eg. structure defined in the public header file).

Quoted 2 lines

Well the distro can just append _v2, _v3 to the name of the function or type if it has to break compat for some reason. Would that be enough?

There are other ways that distros can work around when upstream "breaks" the ABI, sometimes they can rename functions, and others they can "preload" structures with padding in anticipation for when/if fields get added to them. But that's all up to the distros, no need for us to worry about that at all :)

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

Quoted 3 lines

A runtime check is still done, with per-module vermagic which distros can change when they bump the ABI version. Is it really necessary to have more than that (i.e., per-symbol versioning)?

From my point of view, it is. We need to allow changing ABI for some
modules while maintaining it for others.
In fact I think that there should be version not only for every exported symbol (in the EXPORT_SYMBOL() sense), but also for every public type (in the sense of eg. structure defined in the public header file).

Quoted 7 lines

Quoted 2 lines

Well the distro can just append _v2, _v3 to the name of the function or type if it has to break compat for some reason. Would that be enough?

There are other ways that distros can work around when upstream "breaks" the ABI, sometimes they can rename functions, and others they can "preload" structures with padding in anticipation for when/if fields get added to them. But that's all up to the distros, no need for us to worry about that at all :)

Currently, the ABI version (checksum) is stored outside of the actual code in the __ksymtab section. That means that the distributions can still apply upstream patches cleanly and only update the version checksum if these break ABI.

With the _v2, _v3 suffixes (or similar solutions) we'd be effectively storing the ABI versions directly in the code and that would cause conflicts when pulling further patches from upstream.

My view is that it would be than easier to maintain out-of-tree modversions (or similar tool) rather than to solve all these conflicts.

Quoted 24 lines · Nicholas Piggin

Quoted 20 lines · On Fri, 9 Dec 2016 15:36:04 +0100 Stanislav Kozina <[email protected]> wrote:

Quoted 11 lines

Quoted 11 lines

Quoted 4 lines

Quoted 1 line

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

As a tool to aid distro reviewers, modversions has some value, but the debug info parsing tools that have been mentioned in this thread seem superior (not that I've tested them).

On the other hand the big advantage of modversions is that it also verifies the checksum during runtime (module loading). In other words, I believe that any other solution should still generate some form of checksum/watermark which can be easily checked for compatibility on module load.
It should not be hard to add to the DWARF based tools though. We'd just parse DWARF data instead of the C code.

Quoted 3 lines

A runtime check is still done, with per-module vermagic which distros can change when they bump the ABI version. Is it really necessary to have more than that (i.e., per-symbol versioning)?

From my point of view, it is. We need to allow changing ABI for some
modules while maintaining it for others.
In fact I think that there should be version not only for every exported symbol (in the EXPORT_SYMBOL() sense), but also for every public type (in the sense of eg. structure defined in the public header file).

Quoted 2 lines

Well the distro can just append _v2, _v3 to the name of the function or type if it has to break compat for some reason. Would that be enough?

There are other ways that distros can work around when upstream "breaks" the ABI, sometimes they can rename functions, and others they can "preload" structures with padding in anticipation for when/if fields get added to them. But that's all up to the distros, no need for us to worry about that at all :)

Quoted 25 lines · Greg Kroah-Hartman

Quoted 24 lines · Nicholas Piggin

Quoted 20 lines · On Fri, 9 Dec 2016 15:36:04 +0100 Stanislav Kozina <[email protected]> wrote:

Quoted 11 lines

Quoted 11 lines

Quoted 4 lines

Quoted 1 line

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

As a tool to aid distro reviewers, modversions has some value, but the debug info parsing tools that have been mentioned in this thread seem superior (not that I've tested them).

On the other hand the big advantage of modversions is that it also verifies the checksum during runtime (module loading). In other words, I believe that any other solution should still generate some form of checksum/watermark which can be easily checked for compatibility on module load.
It should not be hard to add to the DWARF based tools though. We'd just parse DWARF data instead of the C code.

Quoted 3 lines

A runtime check is still done, with per-module vermagic which distros can change when they bump the ABI version. Is it really necessary to have more than that (i.e., per-symbol versioning)?

From my point of view, it is. We need to allow changing ABI for some
modules while maintaining it for others.
In fact I think that there should be version not only for every exported symbol (in the EXPORT_SYMBOL() sense), but also for every public type (in the sense of eg. structure defined in the public header file).

Quoted 8 lines

Quoted 2 lines

Well the distro can just append _v2, _v3 to the name of the function or type if it has to break compat for some reason. Would that be enough?

There are other ways that distros can work around when upstream "breaks" the ABI, sometimes they can rename functions, and others they can "preload" structures with padding in anticipation for when/if fields get added to them. But that's all up to the distros, no need for us to worry about that at all :)

The _v2 and _v3 functions are probably the ones that also get used by future backports in the distro kernel itself and are probably the reason for the ABI change in the first place. Thus going down this route will basically require distros to touch every future backport patch and will in general generate a big mess internally.

I think it is important to keep versioning information outside of the source code. Some kind of modversions will still be required, but distros should be able to decide if they put in some kind of checksum or a string, what suites them most.

Quoted 35 lines · On Wed, 14 Dec 2016 15:04:36 +0100 Hannes Frederic Sowa <[email protected]>…

Quoted 25 lines · Greg Kroah-Hartman

Quoted 24 lines · Nicholas Piggin

Quoted 20 lines · On Fri, 9 Dec 2016 15:36:04 +0100 Stanislav Kozina <[email protected]> wrote:

Quoted 11 lines

Quoted 11 lines

Quoted 4 lines

Quoted 1 line

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

As a tool to aid distro reviewers, modversions has some value, but the debug info parsing tools that have been mentioned in this thread seem superior (not that I've tested them).

On the other hand the big advantage of modversions is that it also verifies the checksum during runtime (module loading). In other words, I believe that any other solution should still generate some form of checksum/watermark which can be easily checked for compatibility on module load.
It should not be hard to add to the DWARF based tools though. We'd just parse DWARF data instead of the C code.

Quoted 3 lines

A runtime check is still done, with per-module vermagic which distros can change when they bump the ABI version. Is it really necessary to have more than that (i.e., per-symbol versioning)?

From my point of view, it is. We need to allow changing ABI for some
modules while maintaining it for others.
In fact I think that there should be version not only for every exported symbol (in the EXPORT_SYMBOL() sense), but also for every public type (in the sense of eg. structure defined in the public header file).

Quoted 8 lines

Quoted 2 lines

Well the distro can just append _v2, _v3 to the name of the function or type if it has to break compat for some reason. Would that be enough?

There are other ways that distros can work around when upstream "breaks" the ABI, sometimes they can rename functions, and others they can "preload" structures with padding in anticipation for when/if fields get added to them. But that's all up to the distros, no need for us to worry about that at all :)

Quoted 5 lines

The _v2 and _v3 functions are probably the ones that also get used by future backports in the distro kernel itself and are probably the reason for the ABI change in the first place. Thus going down this route will basically require distros to touch every future backport patch and will in general generate a big mess internally.

What kind of big mess? You have to check the logic of each backport even if it does apply cleanly, so the added overhead of the name change should be relatively tiny, no?

Quoted 4 lines

I think it is important to keep versioning information outside of the source code. Some kind of modversions will still be required, but distros should be able to decide if they put in some kind of checksum or a string, what suites them most.

The module crc symbols are just an integer that requires a match, so it could easily be populated by a list that the distro keeps, rather than by genksyms. Most of the complexity is on the build side, so that would still be an improvement for the kernel. So we *could* do this if the distros need it.

Quoted 44 lines · Nicholas Piggin

Quoted 35 lines · On Wed, 14 Dec 2016 15:04:36 +0100 Hannes Frederic Sowa <[email protected]>…

Quoted 25 lines · Greg Kroah-Hartman

Quoted 24 lines · Nicholas Piggin

Quoted 20 lines · On Fri, 9 Dec 2016 15:36:04 +0100 Stanislav Kozina <[email protected]> wrote:

Quoted 11 lines

Quoted 11 lines

Quoted 4 lines

Quoted 1 line

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

As a tool to aid distro reviewers, modversions has some value, but the debug info parsing tools that have been mentioned in this thread seem superior (not that I've tested them).

On the other hand the big advantage of modversions is that it also verifies the checksum during runtime (module loading). In other words, I believe that any other solution should still generate some form of checksum/watermark which can be easily checked for compatibility on module load.
It should not be hard to add to the DWARF based tools though. We'd just parse DWARF data instead of the C code.

Quoted 3 lines

A runtime check is still done, with per-module vermagic which distros can change when they bump the ABI version. Is it really necessary to have more than that (i.e., per-symbol versioning)?

From my point of view, it is. We need to allow changing ABI for some
modules while maintaining it for others.
In fact I think that there should be version not only for every exported symbol (in the EXPORT_SYMBOL() sense), but also for every public type (in the sense of eg. structure defined in the public header file).

Quoted 8 lines

Quoted 2 lines

Well the distro can just append _v2, _v3 to the name of the function or type if it has to break compat for some reason. Would that be enough?

There are other ways that distros can work around when upstream "breaks" the ABI, sometimes they can rename functions, and others they can "preload" structures with padding in anticipation for when/if fields get added to them. But that's all up to the distros, no need for us to worry about that at all :)

Quoted 5 lines

The _v2 and _v3 functions are probably the ones that also get used by future backports in the distro kernel itself and are probably the reason for the ABI change in the first place. Thus going down this route will basically require distros to touch every future backport patch and will in general generate a big mess internally.

Quoted 3 lines

What kind of big mess? You have to check the logic of each backport even if it does apply cleanly, so the added overhead of the name change should be relatively tiny, no?

Basically single patches are backported in huge series. Reviewing each single patch also definitely makes sense, a review of the series as a whole is much more worthwhile because it focuses more on logic.

The patches themselves are checked by individual robots or humans against merge conflict introduced mistakes which ring alarm bells for people to look more closely during review.

Merge conflicts introduced mistakes definitely can happen because developers/backporters lose the focus from the actual logic but deal with shifting lines around or just fixing up postfixes to function names.

We still try to align the kernel as much as possible with upstream, because most developers can't really hold the differences between upstream and the internal functions in their heads (is this function RMW safe in this version but not that kernel version...).

Anyway, I don't think we will at any time have multiple versions of a function exported to 3rd party kernel modules. The headaches are just too big. Basically we would have to version structs and not functions (this is our bigger problem), thus exporting new versions of functions don't really help at all. Having multiple versions of structs really scares me. ;)

We already pad structs to allow for additional struct members to be added, which helps a lot.

If versioning of function symbols would be an issue we probably would have switched to ELF function versioning (like glibc does it) long time ago.

Quoted 10 lines

Quoted 4 lines

I think it is important to keep versioning information outside of the source code. Some kind of modversions will still be required, but distros should be able to decide if they put in some kind of checksum or a string, what suites them most.

The module crc symbols are just an integer that requires a match, so it could easily be populated by a list that the distro keeps, rather than by genksyms. Most of the complexity is on the build side, so that would still be an improvement for the kernel. So we *could* do this if the distros need it.

Like Don also already said, genksyms already did a pretty good job so far. We are right now working with Dodji to come up with a way to replace genksyms, in case people really want to have very specific control about what causes the symbol version to be changed.

Also I wonder what Ben's opinion on this is.. As I understood that he wants to maintain a super-long-term stable kernel with kabi guarantees.

Note, what we want is to weaken the check for kabi, by excluding parts of the struct from genksyms with libabigail. For Red Hat genksyms is too strict in the checks.

Quoted 57 lines · On Thu, 15 Dec 2016 12:19:02 +0100 Hannes Frederic Sowa <[email protected]>…

Quoted 44 lines · Nicholas Piggin

Quoted 35 lines · On Wed, 14 Dec 2016 15:04:36 +0100 Hannes Frederic Sowa <[email protected]>…

Quoted 25 lines · Greg Kroah-Hartman

Quoted 24 lines · Nicholas Piggin

Quoted 20 lines · On Fri, 9 Dec 2016 15:36:04 +0100 Stanislav Kozina <[email protected]> wrote:

Quoted 11 lines

Quoted 11 lines

Quoted 4 lines

Quoted 1 line

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

As a tool to aid distro reviewers, modversions has some value, but the debug info parsing tools that have been mentioned in this thread seem superior (not that I've tested them).

On the other hand the big advantage of modversions is that it also verifies the checksum during runtime (module loading). In other words, I believe that any other solution should still generate some form of checksum/watermark which can be easily checked for compatibility on module load.
It should not be hard to add to the DWARF based tools though. We'd just parse DWARF data instead of the C code.

Quoted 3 lines

A runtime check is still done, with per-module vermagic which distros can change when they bump the ABI version. Is it really necessary to have more than that (i.e., per-symbol versioning)?

From my point of view, it is. We need to allow changing ABI for some
modules while maintaining it for others.
In fact I think that there should be version not only for every exported symbol (in the EXPORT_SYMBOL() sense), but also for every public type (in the sense of eg. structure defined in the public header file).

Quoted 8 lines

Quoted 2 lines

Well the distro can just append _v2, _v3 to the name of the function or type if it has to break compat for some reason. Would that be enough?

There are other ways that distros can work around when upstream "breaks" the ABI, sometimes they can rename functions, and others they can "preload" structures with padding in anticipation for when/if fields get added to them. But that's all up to the distros, no need for us to worry about that at all :)

Quoted 5 lines

The _v2 and _v3 functions are probably the ones that also get used by future backports in the distro kernel itself and are probably the reason for the ABI change in the first place. Thus going down this route will basically require distros to touch every future backport patch and will in general generate a big mess internally.

Quoted 3 lines

What kind of big mess? You have to check the logic of each backport even if it does apply cleanly, so the added overhead of the name change should be relatively tiny, no?

Basically single patches are backported in huge series. Reviewing each single patch also definitely makes sense, a review of the series as a whole is much more worthwhile because it focuses more on logic.

The patches themselves are checked by individual robots or humans against merge conflict introduced mistakes which ring alarm bells for people to look more closely during review.

Quoted 8 lines

Merge conflicts introduced mistakes definitely can happen because developers/backporters lose the focus from the actual logic but deal with shifting lines around or just fixing up postfixes to function names.

We still try to align the kernel as much as possible with upstream, because most developers can't really hold the differences between upstream and the internal functions in their heads (is this function RMW safe in this version but not that kernel version...).

I agree with all this, but in the case of a function rename, you can automate it all with scripts if that's what you want.

When you have your list of exported symbols with non-zero version number, then you can script that __abivXXX into the changeset applying process, or alternatively apply the rename after your patches are applied, or use the c preprocessor to define names to something else.

Quoted 24 lines

Anyway, I don't think we will at any time have multiple versions of a function exported to 3rd party kernel modules. The headaches are just too big. Basically we would have to version structs and not functions (this is our bigger problem), thus exporting new versions of functions don't really help at all. Having multiple versions of structs really scares me. ;)

We already pad structs to allow for additional struct members to be added, which helps a lot.

If versioning of function symbols would be an issue we probably would have switched to ELF function versioning (like glibc does it) long time ago.

Quoted 10 lines

Quoted 4 lines

I think it is important to keep versioning information outside of the source code. Some kind of modversions will still be required, but distros should be able to decide if they put in some kind of checksum or a string, what suites them most.

The module crc symbols are just an integer that requires a match, so it could easily be populated by a list that the distro keeps, rather than by genksyms. Most of the complexity is on the build side, so that would still be an improvement for the kernel. So we *could* do this if the distros need it.

Quoted 4 lines

Like Don also already said, genksyms already did a pretty good job so far. We are right now working with Dodji to come up with a way to replace genksyms, in case people really want to have very specific control about what causes the symbol version to be changed.

Yeah it's great work, so is Stanislav's checker. I wouldn't mind having a kernel-centric checker tool merged in the kernel if it is small, maintained, and does a sufficient job for distros.

Quoted 6 lines

Also I wonder what Ben's opinion on this is.. As I understood that he wants to maintain a super-long-term stable kernel with kabi guarantees.

Note, what we want is to weaken the check for kabi, by excluding parts of the struct from genksyms with libabigail. For Red Hat genksyms is too strict in the checks.

Sure, that makes sense.

So if I understand where we are, moving the ABI compatibility checking to one of these tools looks possible. What to do when we have an ABI change is not settled, but feeding version numbers explicitly into modversions is an option that would be close to what distros do today.

Quoted 69 lines · Nicholas Piggin

Quoted 57 lines · On Thu, 15 Dec 2016 12:19:02 +0100 Hannes Frederic Sowa <[email protected]>…

Quoted 44 lines · Nicholas Piggin

Quoted 35 lines · On Wed, 14 Dec 2016 15:04:36 +0100 Hannes Frederic Sowa <[email protected]>…

Quoted 25 lines · Greg Kroah-Hartman

Quoted 24 lines · Nicholas Piggin

Quoted 20 lines · On Fri, 9 Dec 2016 15:36:04 +0100 Stanislav Kozina <[email protected]> wrote:

Quoted 11 lines

Quoted 11 lines

Quoted 4 lines

Quoted 1 line

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

As a tool to aid distro reviewers, modversions has some value, but the debug info parsing tools that have been mentioned in this thread seem superior (not that I've tested them).

On the other hand the big advantage of modversions is that it also verifies the checksum during runtime (module loading). In other words, I believe that any other solution should still generate some form of checksum/watermark which can be easily checked for compatibility on module load.
It should not be hard to add to the DWARF based tools though. We'd just parse DWARF data instead of the C code.

Quoted 3 lines

A runtime check is still done, with per-module vermagic which distros can change when they bump the ABI version. Is it really necessary to have more than that (i.e., per-symbol versioning)?

From my point of view, it is. We need to allow changing ABI for some
modules while maintaining it for others.
In fact I think that there should be version not only for every exported symbol (in the EXPORT_SYMBOL() sense), but also for every public type (in the sense of eg. structure defined in the public header file).

Quoted 8 lines

Quoted 2 lines

Well the distro can just append _v2, _v3 to the name of the function or type if it has to break compat for some reason. Would that be enough?

There are other ways that distros can work around when upstream "breaks" the ABI, sometimes they can rename functions, and others they can "preload" structures with padding in anticipation for when/if fields get added to them. But that's all up to the distros, no need for us to worry about that at all :)

Quoted 5 lines

The _v2 and _v3 functions are probably the ones that also get used by future backports in the distro kernel itself and are probably the reason for the ABI change in the first place. Thus going down this route will basically require distros to touch every future backport patch and will in general generate a big mess internally.

Quoted 3 lines

What kind of big mess? You have to check the logic of each backport even if it does apply cleanly, so the added overhead of the name change should be relatively tiny, no?

Basically single patches are backported in huge series. Reviewing each single patch also definitely makes sense, a review of the series as a whole is much more worthwhile because it focuses more on logic.

The patches themselves are checked by individual robots or humans against merge conflict introduced mistakes which ring alarm bells for people to look more closely during review.

Quoted 8 lines

Merge conflicts introduced mistakes definitely can happen because developers/backporters lose the focus from the actual logic but deal with shifting lines around or just fixing up postfixes to function names.

We still try to align the kernel as much as possible with upstream, because most developers can't really hold the differences between upstream and the internal functions in their heads (is this function RMW safe in this version but not that kernel version...).

Quoted 7 lines

I agree with all this, but in the case of a function rename, you can automate it all with scripts if that's what you want.

When you have your list of exported symbols with non-zero version number, then you can script that __abivXXX into the changeset applying process, or alternatively apply the rename after your patches are applied, or use the c preprocessor to define names to something else.

Yes, probably one could come up with coccinelle patches to do this, preprocessing/string matching could have false positives. But as I wrote above, we need one stable ABI and not multiple for our particular kernels, so it seems like a lot of overhead to rename particular functions internally all the time to make them inaccessible for external modules.

Quoted 8 lines

Quoted 4 lines

Like Don also already said, genksyms already did a pretty good job so far. We are right now working with Dodji to come up with a way to replace genksyms, in case people really want to have very specific control about what causes the symbol version to be changed.

Yeah it's great work, so is Stanislav's checker. I wouldn't mind having a kernel-centric checker tool merged in the kernel if it is small, maintained, and does a sufficient job for distros.

Yes, I think this needs more experimentation and thought right now before we can make a decision.

Quoted 7 lines

Quoted 6 lines

Also I wonder what Ben's opinion on this is.. As I understood that he wants to maintain a super-long-term stable kernel with kabi guarantees.

Note, what we want is to weaken the check for kabi, by excluding parts of the struct from genksyms with libabigail. For Red Hat genksyms is too strict in the checks.

Quoted 6 lines

Sure, that makes sense.

So if I understand where we are, moving the ABI compatibility checking to one of these tools looks possible. What to do when we have an ABI change is not settled, but feeding version numbers explicitly into modversions is an option that would be close to what distros do today.

Agreed!

Quoted 78 lines · On Thu, 15 Dec 2016 14:15:31 +0100 Hannes Frederic Sowa <[email protected]>…

Quoted 69 lines · Nicholas Piggin

Quoted 57 lines · On Thu, 15 Dec 2016 12:19:02 +0100 Hannes Frederic Sowa <[email protected]>…

Quoted 44 lines · Nicholas Piggin

Quoted 35 lines · On Wed, 14 Dec 2016 15:04:36 +0100 Hannes Frederic Sowa <[email protected]>…

Quoted 25 lines · Greg Kroah-Hartman

Quoted 24 lines · Nicholas Piggin

Quoted 20 lines · On Fri, 9 Dec 2016 15:36:04 +0100 Stanislav Kozina <[email protected]> wrote:

Quoted 11 lines

Quoted 11 lines

Quoted 4 lines

Quoted 1 line

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

As a tool to aid distro reviewers, modversions has some value, but the debug info parsing tools that have been mentioned in this thread seem superior (not that I've tested them).

On the other hand the big advantage of modversions is that it also verifies the checksum during runtime (module loading). In other words, I believe that any other solution should still generate some form of checksum/watermark which can be easily checked for compatibility on module load.
It should not be hard to add to the DWARF based tools though. We'd just parse DWARF data instead of the C code.

Quoted 3 lines

A runtime check is still done, with per-module vermagic which distros can change when they bump the ABI version. Is it really necessary to have more than that (i.e., per-symbol versioning)?

From my point of view, it is. We need to allow changing ABI for some
modules while maintaining it for others.
In fact I think that there should be version not only for every exported symbol (in the EXPORT_SYMBOL() sense), but also for every public type (in the sense of eg. structure defined in the public header file).

Quoted 8 lines

Quoted 2 lines

Well the distro can just append _v2, _v3 to the name of the function or type if it has to break compat for some reason. Would that be enough?

There are other ways that distros can work around when upstream "breaks" the ABI, sometimes they can rename functions, and others they can "preload" structures with padding in anticipation for when/if fields get added to them. But that's all up to the distros, no need for us to worry about that at all :)

Quoted 5 lines

The _v2 and _v3 functions are probably the ones that also get used by future backports in the distro kernel itself and are probably the reason for the ABI change in the first place. Thus going down this route will basically require distros to touch every future backport patch and will in general generate a big mess internally.

Quoted 3 lines

What kind of big mess? You have to check the logic of each backport even if it does apply cleanly, so the added overhead of the name change should be relatively tiny, no?

Basically single patches are backported in huge series. Reviewing each single patch also definitely makes sense, a review of the series as a whole is much more worthwhile because it focuses more on logic.

The patches themselves are checked by individual robots or humans against merge conflict introduced mistakes which ring alarm bells for people to look more closely during review.

Quoted 8 lines

Merge conflicts introduced mistakes definitely can happen because developers/backporters lose the focus from the actual logic but deal with shifting lines around or just fixing up postfixes to function names.

We still try to align the kernel as much as possible with upstream, because most developers can't really hold the differences between upstream and the internal functions in their heads (is this function RMW safe in this version but not that kernel version...).

Quoted 7 lines

I agree with all this, but in the case of a function rename, you can automate it all with scripts if that's what you want.

When you have your list of exported symbols with non-zero version number, then you can script that __abivXXX into the changeset applying process, or alternatively apply the rename after your patches are applied, or use the c preprocessor to define names to something else.

Quoted 6 lines

Yes, probably one could come up with coccinelle patches to do this, preprocessing/string matching could have false positives. But as I wrote above, we need one stable ABI and not multiple for our particular kernels, so it seems like a lot of overhead to rename particular functions internally all the time to make them inaccessible for external modules.

I can't be sure until it's implemented in a workflow that distros are happy with of course, but I just don't see why it would be a lot of overhead. Particularly if you just scripted everything.

How frequently do symbols become incompatible within an ostensibly ABI- stable release?

Quoted 9 lines

Quoted 8 lines

Quoted 4 lines

Like Don also already said, genksyms already did a pretty good job so far. We are right now working with Dodji to come up with a way to replace genksyms, in case people really want to have very specific control about what causes the symbol version to be changed.

Yeah it's great work, so is Stanislav's checker. I wouldn't mind having a kernel-centric checker tool merged in the kernel if it is small, maintained, and does a sufficient job for distros.

Quoted 2 lines

Yes, I think this needs more experimentation and thought right now before we can make a decision.

Sure, I wanted to mention it in case people had a concern about out of tree tools. It will depend on what distros end up settling with.

Yeah it's great work, so is Stanislav's checker. I wouldn't mind having a kernel-centric checker tool merged in the kernel if it is small, maintained, and does a sufficient job for distros.

So if I understand where we are, moving the ABI compatibility checking to one of these tools looks possible. What to do when we have an ABI change is not settled, but feeding version numbers explicitly into modversions is an option that would be close to what distros do today.

Quoted 7 lines

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

Quoted 1 line

Scary, huh? :-)

Oh yeah my aim here is not to make distro or out of tree module vendors life harder, actually the opposite. If it turns out modversions really is the best approach, I'm not in a position to complain about its complexity because we have Suse and Redhat people maintaining the build and module systems :) I just want to see if we can do things better.

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

On 12/01/2016 05:13 AM, Don Zickus wrote:

...

Quoted 2 lines

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

I should mention one of the others here:
https://github.com/skozina/kabi-dw

It's quite comparable to libabigail in the way it works, the main differences are:
- written in pure C
- depends only on elf-utils and flex/yacc
- it's much simpler (4k LOC)
- stores the type information in the text files and compares those
instead of directly comparing two sets of DWARF data

Quoted 9 lines · On Thu, 1 Dec 2016 11:48:09 +0100 Stanislav Kozina <[email protected]> wrote:

On 12/01/2016 05:13 AM, Don Zickus wrote:

...

Quoted 2 lines

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

I should mention one of the others here:
https://github.com/skozina/kabi-dw

Quoted 7 lines

It's quite comparable to libabigail in the way it works, the main differences are:
- written in pure C
- depends only on elf-utils and flex/yacc
- it's much simpler (4k LOC)
- stores the type information in the text files and compares those
instead of directly comparing two sets of DWARF data

Now this seems much better for distro ABI checking.

The next question is, do they need any kernel support for rare cases where they do have to break the ABI of an export? Simple rename of the function with a _v2 postfix might be enough. We could retain some per symbol versioning in the kernel if needed, but how much would it actually help?

Quoted 21 lines · Nicholas Piggin

Quoted 9 lines · On Thu, 1 Dec 2016 11:48:09 +0100 Stanislav Kozina <[email protected]> wrote:

On 12/01/2016 05:13 AM, Don Zickus wrote:

...

Quoted 2 lines

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

I should mention one of the others here:
https://github.com/skozina/kabi-dw

Quoted 7 lines

It's quite comparable to libabigail in the way it works, the main differences are:
- written in pure C
- depends only on elf-utils and flex/yacc
- it's much simpler (4k LOC)
- stores the type information in the text files and compares those
instead of directly comparing two sets of DWARF data

Now this seems much better for distro ABI checking.

Quoted 5 lines

The next question is, do they need any kernel support for rare cases where they do have to break the ABI of an export? Simple rename of the function with a _v2 postfix might be enough. We could retain some per symbol versioning in the kernel if needed, but how much would it actually help?

The biggest pain point AFAICT is to identify what types (functions, structs, enums, ...) should be considered a part of the stable ABI.

And
the problem with modversions is that it pulls in just everything which gets (accidentally?) #included in the source file.

The actual ABI maintenance is a different problem, but there are many possible approaches, the _v2 suffix being one of them.

Quoted 9 lines · On Thu, 1 Dec 2016 11:48:09 +0100 Stanislav Kozina <[email protected]> wrote:

On 12/01/2016 05:13 AM, Don Zickus wrote:

...

Quoted 2 lines

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

I should mention one of the others here:
https://github.com/skozina/kabi-dw

Now this seems much better for distro ABI checking.

The next question is, do they need any kernel support for rare cases where they do have to break the ABI of an export? Simple rename of the function with a _v2 postfix might be enough. We could retain some per symbol versioning in the kernel if needed, but how much would it actually help?

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

Quoted 4 lines · Adam Borowski

Quoted 2 lines

(a) tested

By many people.

No.

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

Thus, if it's indeed binutils, you'll see the breakage as soon as Fedora recovers from the freeze.

So quite frankly, I don't want to make our kernel sources worse due to broken shit tools getting something wrong that we shouldn't even care about.

Quoted 4 lines · Linus Torvalds

So quite frankly, I don't want to make our kernel sources worse due to broken shit tools getting something wrong that we shouldn't even care about.

And yes, I'm on binutils 2.26 (with no issues), so it could be that it's 2.27 that triggers this.

We could make the pr_warn_once() mention "broken binutils?" so that people know why the warning happens.

Quoted 7 lines · CET Linus Torvalds

Quoted 4 lines · Linus Torvalds

So quite frankly, I don't want to make our kernel sources worse due to broken shit tools getting something wrong that we shouldn't even care about.

Quoted 5 lines

And yes, I'm on binutils 2.26 (with no issues), so it could be that it's 2.27 that triggers this.

We could make the pr_warn_once() mention "broken binutils?" so that people know why the warning happens.

I've tried to get to the bottom of this, but couldn't find anything related to the toolchain version. I've tried binutils 2.23, 2.24, 2.26 and 2.27, and also gcc-7.0, gcc-5.4.1 and gcc-4.9.3, but with today's linux-next, I always get

WARNING: EXPORT symbol "mcount" [arch/x86/entry/built-in.ko] version generation failed, symbol will not be versioned.
WARNING: EXPORT symbol "mcount" [arch/x86/built-in.ko] version generation failed, symbol will not be versioned.
WARNING: EXPORT symbol "cmpxchg8b_emu" [vmlinux] version generation failed, symbol will not be versioned.
WARNING: EXPORT symbol "empty_zero_page" [vmlinux] version generation failed, symbol will not be versioned.
WARNING: EXPORT symbol "mcount" [vmlinux] version generation failed, symbol will not be versioned.
WARNING: EXPORT symbol "cmpxchg8b_emu" [vmlinux] version generation failed, symbol will not be versioned.
WARNING: EXPORT symbol "empty_zero_page" [vmlinux] version generation failed, symbol will not be versioned.
WARNING: EXPORT symbol "mcount" [vmlinux] version generation failed, symbol will not be versioned.

Out of 12 randconfig builds that had CONFIG_MODVERSIONS enabled, all 12 had this problem, though not always with all the symbols.

WARNING: EXPORT symbol "mcount" [arch/x86/entry/built-in.ko] version generation failed, symbol will not be versioned.
WARNING: EXPORT symbol "mcount" [arch/x86/built-in.ko] version generation failed, symbol will not be versioned.
WARNING: EXPORT symbol "cmpxchg8b_emu" [vmlinux] version generation failed, symbol will not be versioned.
WARNING: EXPORT symbol "empty_zero_page" [vmlinux] version generation failed, symbol will not be versioned.
WARNING: EXPORT symbol "mcount" [vmlinux] version generation failed, symbol will not be versioned.
WARNING: EXPORT symbol "cmpxchg8b_emu" [vmlinux] version generation failed, symbol will not be versioned.
WARNING: EXPORT symbol "empty_zero_page" [vmlinux] version generation failed, symbol will not be versioned.
WARNING: EXPORT symbol "mcount" [vmlinux] version generation failed, symbol will not be versioned.

Out of 12 randconfig builds that had CONFIG_MODVERSIONS enabled, all 12 had this problem, though not always with all the symbols.

Quoted 10 lines · Arnd Bergmann

WARNING: EXPORT symbol "mcount" [arch/x86/entry/built-in.ko] version generation failed, symbol will not be versioned.
WARNING: EXPORT symbol "mcount" [arch/x86/built-in.ko] version generation failed, symbol will not be versioned.
WARNING: EXPORT symbol "cmpxchg8b_emu" [vmlinux] version generation failed, symbol will not be versioned.
WARNING: EXPORT symbol "empty_zero_page" [vmlinux] version generation failed, symbol will not be versioned.
WARNING: EXPORT symbol "mcount" [vmlinux] version generation failed, symbol will not be versioned.
WARNING: EXPORT symbol "cmpxchg8b_emu" [vmlinux] version generation failed, symbol will not be versioned.
WARNING: EXPORT symbol "empty_zero_page" [vmlinux] version generation failed, symbol will not be versioned.
WARNING: EXPORT symbol "mcount" [vmlinux] version generation failed, symbol will not be versioned.

Quoted 2 lines

Out of 12 randconfig builds that had CONFIG_MODVERSIONS enabled, all 12 had this problem, though not always with all the symbols.

Well, the good news is that pretty fundamentally, if it's just the asm symbls, those really don't have ABI's that change (or if they change, it's such a fundamental change that everything else will likely have changed too and we don't need to worry about one asm symbol crc - the change will be caught by other symbols).

So I think the whole "we don't really care" approach should work fine. The "let's make every symbol always be versioned" may just be too much pain for no real gain.

With binutils-2.16 and before, a weak missing symbol was kept during the

final link, and a missing CRC for an export would lead to that CRC being treated as zero implicitly. With binutils-2.17, the crc

symbol gets dropped, and any module trying to use it will fail to load.

Quoted 1 line · Arnd Bergmann

With binutils-2.16 and before, a weak missing symbol was kept during the

2.26?

Quoted 2 lines

final link, and a missing CRC for an export would lead to that CRC being treated as zero implicitly. With binutils-2.17, the crc

2.27?'

With binutils-2.16 and before, a weak missing symbol was kept during the final link, and a missing CRC for an export would lead to that CRC being treated as zero implicitly. With binutils-2.17, the crc symbol gets dropped, and any module trying to use it will fail to load.

This sets the weak CRC symbol to zero explicitly, making it defined in vmlinux, which in turn lets us load the modules referring to that CRC.

The comment above the __CRC_SYMBOL macro suggests that this was always the intention, although it also seems that all symbols defined in C have a correct CRC these days, and only the exports that are now done in assembly need this.

Signed-off-by Arnd Bergmann <[email protected]>

---

include/asm-generic/export.h +1 -0

index 63554e9..59a3b2f 100644
@@ -54,6 +54,7 @@ KSYM(__kstrtab_\name):
 KSYM(__kcrctab_\name):
 	__put KSYM(__crc_\name)
 	.weak KSYM(__crc_\name)
+	.set KSYM(__crc_\name), 0
 	.previous
 #endif
 #endif

With binutils-2.16 and before, a weak missing symbol was kept during the final link, and a missing CRC for an export would lead to that CRC being treated as zero implicitly. With binutils-2.17, the crc symbol gets dropped, and any module trying to use it will fail to load.

This sets the weak CRC symbol to zero explicitly, making it defined in vmlinux, which in turn lets us load the modules referring to that CRC.

The comment above the __CRC_SYMBOL macro suggests that this was always the intention, although it also seems that all symbols defined in C have a correct CRC these days, and only the exports that are now done in assembly need this.

Quoted 1 line

Signed-off-by Arnd Bergmann <[email protected]>

---
Not sure if this is the correct way of doing it, but this seems trivial enough and lets me build the kernel with missing CRCs with any binutils version.

include/asm-generic/export.h +0 -0

index 63554e9..59a3b2f 100644
@@ -54,6 +54,7 @@ KSYM(__kstrtab_\name):

 KSYM(__kcrctab_\name):

Quoted 4 lines

  __put KSYM(__crc_\name)
  .weak KSYM(__crc_\name)

include/asm-generic/export.h +1 -0

+	.set KSYM(__crc_\name), 0

  .previous

 #endif
 #endif

Quoted 15 lines · Arnd Bergmann

With binutils-2.16 and before, a weak missing symbol was kept during the final link, and a missing CRC for an export would lead to that CRC being treated as zero implicitly. With binutils-2.17, the crc symbol gets dropped, and any module trying to use it will fail to load.

This sets the weak CRC symbol to zero explicitly, making it defined in vmlinux, which in turn lets us load the modules referring to that CRC.

The comment above the __CRC_SYMBOL macro suggests that this was always the intention, although it also seems that all symbols defined in C have a correct CRC these days, and only the exports that are now done in assembly need this.

Quoted 5 lines

Quoted 1 line

Signed-off-by Arnd Bergmann <[email protected]>

---
Not sure if this is the correct way of doing it, but this seems trivial enough and lets me build the kernel with missing CRCs with any binutils version.

I tried this along with Adam's patch on x86_64, with Debian's binutils 2.27.51.20161127. The result was that the kernel's __kcrctab held 0 for several symbols, even though there was type information in asm- prototypes.h and Module.symvers and the modules had a non-zero CRC for those symbols. With just Adam's patch, the kernel and modules agreed.

Yes, it's always been just the assembly symbols that broke, these were the ones that Al's original patch changed and that ended up with no version information.

I have managed to bisect the link failure to a specific binutils commit by Alan Modra now:

I have managed to bisect the link failure to a specific binutils commit by Alan Modra now:

d983c8c ("Strip undefined symbols from .symtab")

went into binutils-2_26 and was reverted in

a82e3ef ("Revert "Strip undefined symbols from .symtab"")

after the release branch for 2.26 was started, so that version ended up being fine. However, the 2.27 version never saw the revert and causes loadable kernel modules to become unusable when they refer to a weak symbol in vmlinux. This works with 2.26 and lower:

As far as reverting the binutils commit goes, I'm quite willing to do that if necessary

How about this stupid patch? It weakens modversions, but that may be ok for Debian, and a better alternative than just saying "we don't support it at all".

Quoted 20 lines · Adam Borowski

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 <[email protected]>

Tested-by Kalle Valo <[email protected]>

Acked-by Nicholas Piggin <[email protected]>

Tested-by Peter Wu <[email protected]>

Tested-by Oliver Hartkopp <[email protected]>

---
Changes: corrected Peter Wu's address, added Tested-by: Oliver. This is an unsplit version (x86/include/ and include/ together).

Quoted 5 lines

 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 <[email protected]>
AuthorDate: Tue Nov 1 12:46:19 2016 +1100
Commit: Michal Marek <[email protected]>
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'!

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 <[email protected]>

Tested-by Kalle Valo <[email protected]>

Acked-by Nicholas Piggin <[email protected]>

Tested-by Peter Wu <[email protected]>

Tested-by Oliver Hartkopp <[email protected]>

Last version (rewritten description) is at:
https://patchwork.kernel.org/patch/9439501/
(needs s/oeter/peter/ for a typo in Peter Wu's address)

Am 23.11.2016 um 21:53 schrieb Adam Borowski:

Quoted 3 lines

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 <[email protected]>

Tested-by Philip Mueller <[email protected]>

Thx for the patch again.

Greez, Phil