Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1530479 > unrolled thread
| Started by | Linus Torvalds <torvalds@linux-foundation.org> |
|---|---|
| First post | 2016-11-25 19:10 +0100 |
| Last post | 2016-11-29 22:50 +0100 |
| Articles | 20 on this page of 84 — 17 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Linus Torvalds <torvalds@linux-foundation.org> - 2016-11-25 19:10 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Nicholas Piggin <npiggin@gmail.com> - 2016-11-26 02:00 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Ben Hutchings <ben@decadent.org.uk> - 2016-11-29 02:20 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Nicholas Piggin <npiggin@gmail.com> - 2016-11-29 03:40 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Michal Marek <mmarek@suse.com> - 2016-11-29 10:20 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Linus Torvalds <torvalds@linux-foundation.org> - 2016-11-29 05:10 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Adam Borowski <kilobyte@angband.pl> - 2016-11-29 14:20 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Ingo Molnar <mingo@kernel.org> - 2016-11-29 14:40 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Adam Borowski <kilobyte@angband.pl> - 2016-11-29 15:30 +0100
[PATCH] x86/kbuild: enable modversions for symbols exported from asm Adam Borowski <kilobyte@angband.pl> - 2016-11-29 15:00 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Linus Torvalds <torvalds@linux-foundation.org> - 2016-11-29 16:50 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Michal Marek <mmarek@suse.com> - 2016-11-29 17:20 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Linus Torvalds <torvalds@linux-foundation.org> - 2016-11-29 17:40 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Ben Hutchings <ben@decadent.org.uk> - 2016-11-29 21:00 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Linus Torvalds <torvalds@linux-foundation.org> - 2016-11-29 22:00 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Linus Torvalds <torvalds@linux-foundation.org> - 2016-11-30 20:00 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Ben Hutchings <ben@decadent.org.uk> - 2016-11-30 22:40 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Nicholas Piggin <npiggin@gmail.com> - 2016-12-01 03:20 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Ben Hutchings <ben@decadent.org.uk> - 2016-12-01 03:40 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Nicholas Piggin <npiggin@gmail.com> - 2016-12-01 05:00 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Michal Marek <mmarek@suse.com> - 2016-12-01 17:40 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Hannes Frederic Sowa <hannes@stressinduktion.org> - 2016-12-02 16:00 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Nicholas Piggin <npiggin@gmail.com> - 2016-12-09 05:00 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Ian Campbell <ijc@hellion.org.uk> - 2016-12-09 16:30 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Nicholas Piggin <npiggin@gmail.com> - 2016-12-09 17:40 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-12-10 14:00 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Nicholas Piggin <npiggin@gmail.com> - 2016-12-12 05:10 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Ian Campbell <ijc@hellion.org.uk> - 2016-12-12 10:10 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Don Zickus <dzickus@redhat.com> - 2016-12-14 19:20 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Stanislav Kozina <skozina@redhat.com> - 2016-12-13 02:30 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Michal Marek <mmarek@suse.com> - 2016-12-14 00:10 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Dodji Seketeli <dodji@seketeli.org> - 2016-12-14 10:20 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Michal Marek <mmarek@suse.com> - 2016-12-14 10:40 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Michal Marek <mmarek@suse.com> - 2016-12-14 11:00 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Dodji Seketeli <dodji@seketeli.org> - 2016-12-14 11:00 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Michal Marek <mmarek@suse.com> - 2016-12-14 11:10 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Dodji Seketeli <dodji@seketeli.org> - 2016-12-14 11:20 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Michal Marek <mmarek@suse.com> - 2016-12-14 11:40 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Dodji Seketeli <dodji@seketeli.org> - 2016-12-14 11:20 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Don Zickus <dzickus@redhat.com> - 2016-12-01 05:50 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Nicholas Piggin <npiggin@gmail.com> - 2016-12-01 05:50 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Don Zickus <dzickus@redhat.com> - 2016-12-01 16:40 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Don Zickus <dzickus@redhat.com> - 2016-12-01 17:00 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-12-01 17:40 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Don Zickus <dzickus@redhat.com> - 2016-12-01 20:10 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Christoph Hellwig <hch@infradead.org> - 2016-12-01 17:10 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Nicholas Piggin <npiggin@gmail.com> - 2016-12-09 05:10 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Stanislav Kozina <skozina@redhat.com> - 2016-12-09 09:20 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Nicholas Piggin <npiggin@gmail.com> - 2016-12-09 09:40 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Stanislav Kozina <skozina@redhat.com> - 2016-12-09 16:00 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Nicholas Piggin <npiggin@gmail.com> - 2016-12-09 17:20 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-12-09 17:30 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Stanislav Kozina <skozina@redhat.com> - 2016-12-12 11:10 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Nicholas Piggin <npiggin@gmail.com> - 2016-12-13 08:50 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Hannes Frederic Sowa <hannes@redhat.com> - 2016-12-14 15:40 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Nicholas Piggin <npiggin@gmail.com> - 2016-12-15 03:30 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Hannes Frederic Sowa <hannes@redhat.com> - 2016-12-15 12:40 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Nicholas Piggin <npiggin@gmail.com> - 2016-12-15 13:30 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Hannes Frederic Sowa <hannes@redhat.com> - 2016-12-15 14:40 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Nicholas Piggin <npiggin@gmail.com> - 2016-12-15 15:40 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Hannes Frederic Sowa <hannes@redhat.com> - 2016-12-15 16:40 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Stanislav Kozina <skozina@redhat.com> - 2016-12-15 15:00 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Don Zickus <dzickus@redhat.com> - 2016-12-09 17:40 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Stanislav Kozina <skozina@redhat.com> - 2016-12-01 12:20 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Nicholas Piggin <npiggin@gmail.com> - 2016-12-01 12:30 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Stanislav Kozina <skozina@redhat.com> - 2016-12-01 13:00 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Nicholas Piggin <npiggin@gmail.com> - 2016-12-01 14:00 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Dodji Seketeli <dodji@seketeli.org> - 2016-12-01 16:50 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Michal Marek <mmarek@suse.com> - 2016-12-01 17:40 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Adam Borowski <kilobyte@angband.pl> - 2016-11-29 18:10 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Linus Torvalds <torvalds@linux-foundation.org> - 2016-11-29 18:30 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Linus Torvalds <torvalds@linux-foundation.org> - 2016-11-29 18:40 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Arnd Bergmann <arnd@arndb.de> - 2016-12-01 15:20 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Michal Marek <mmarek@suse.com> - 2016-12-01 17:40 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Linus Torvalds <torvalds@linux-foundation.org> - 2016-12-01 19:50 +0100
Re: [RFC, PATCH, v3.9] default exported asm symbols to zero Geert Uytterhoeven <geert@linux-m68k.org> - 2016-12-02 14:00 +0100
Re: [RFC, PATCH, v3.9] default exported asm symbols to zero Arnd Bergmann <arnd@arndb.de> - 2016-12-02 16:10 +0100
Re: [RFC, PATCH, v3.9] default exported asm symbols to zero Adam Borowski <kilobyte@angband.pl> - 2016-12-02 16:40 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Linus Torvalds <torvalds@linux-foundation.org> - 2016-12-02 18:30 +0100
Re: [RFC, PATCH, v3.9] default exported asm symbols to zero Ben Hutchings <ben@decadent.org.uk> - 2016-12-03 05:40 +0100
Re: [RFC, PATCH, v3.9] default exported asm symbols to zero Arnd Bergmann <arnd@arndb.de> - 2016-12-03 12:10 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Alan Modra <amodra@gmail.com> - 2016-12-04 09:20 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Linus Torvalds <torvalds@linux-foundation.org> - 2016-12-04 22:10 +0100
Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm Michal Marek <mmarek@suse.com> - 2016-11-29 22:50 +0100
Page 3 of 5 — ← Prev page 1 2 [3] 4 5 Next page →
| From | Nicholas Piggin <npiggin@gmail.com> |
|---|---|
| Date | 2016-12-01 05:50 +0100 |
| Subject | Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm |
| Message-ID | <sJrk5-7mB-1@gated-at.bofh.it> |
| In reply to | #1533796 |
On Wed, 30 Nov 2016 23:13:25 -0500 Don Zickus <dzickus@redhat.com> wrote: > On Wed, Nov 30, 2016 at 10:40:02AM -0800, Linus Torvalds wrote: > > On Wed, Nov 30, 2016 at 10:18 AM, Nicholas Piggin <npiggin@gmail.com> wrote: > > > > > > Here's an initial rough hack at removing modversions. It gives an idea > > > of the complexity we're carrying for this feature (keeping in mind most > > > of the lines removed are generated parser). > > > > You definitely don't have to try to convince me. We've had many issues > > with modversions over the years. This was just the "last drop" as far > > as I'm concerned, we've had random odd crc generation failures due to > > some build races too. > > > > > In its place I just added a simple config option to override vermagic > > > so distros can manage it entirely themselves. > > > > So at least Fedora doesn't even enable CONFIG_MODVERSIONS as-is. I'm > > _hoping_ it's just Debian that wants this, 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. 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. > I am not sure what 'control vermagic' is, but it sounds like a string check, > which won't protect against the boatload of backports we do to structs, > enums, and functions. Basically vermagic is the string all modules and the kernel get, which must match in order to load modules. If you have modversions disabled, then vermagic includes the kernel version. If modversions is enabled, then vermagic does not include the kernel version but the CRCs have to also match. Controlling it explicitly is just a couple of lines where a distro can control it (so they can update their kernel version without breaking). It's not meant to solve everything, just the first one. > Currently we are exploring various ways to get smarter here. The genksyms > tool has its limitations and handling kabi hacks in RHEL is getting > tiresome. > > I think GregKH pointed to one such tool, libabigail? We are working on > others too. > > > Circling back to enabling MODVERSIONS in Fedora, that was to start the > process of syncing Fedora with RHEL stuff in preparation for smarter tools. > > > If you take away MODVERSIONS, that would put a damper in our work, but > easily carried privately (much like MODSIGNING for 8 years until it went > upstream :-) ). I don't think that's necessary. A feature requirement for a distro is just as valid as any other user of upstream. I don't want to hinder any distro, I'm just still not quite seeing the big picture of exactly what functionality you need from the kernel. Thanks, Nick
[toc] | [prev] | [next] | [standalone]
| From | Don Zickus <dzickus@redhat.com> |
|---|---|
| Date | 2016-12-01 16:40 +0100 |
| Subject | Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm |
| Message-ID | <sJBt7-5DE-19@gated-at.bofh.it> |
| In reply to | #1533798 |
On Thu, Dec 01, 2016 at 03:32:15PM +1100, Nicholas Piggin wrote: > > Anyway, MODVERSIONS is our way of protecting our kabi for the last 10 years. > > It isn't perfect and we have fixed the genksyms tool over the years, but so > > far it mostly works fine. > > Okay. It would be good to get all the distros in on this. > > What I want to do is work out exactly what it is that modversions is > giving you. > > We know it's fairly nasty code to maintain and it does not detect ABI > changes very well. But it's not such a burden that we can't maintain > it if there are good reasons to keep it. Hi Nick, I won't disagree with you there. :-) modversions is a pretty heavy handed approach that basically says if all the symbols and types haven't changed for a given EXPORT_SYMBOL (recursively checked), then there is a high degree of confidence the OOT driver will not only load, but run correctly. The question is how to provide a similar guarantee if a different way? We have plenty of customers with 10 year old drivers, where the expertise has long left the company. The engineers still around, recompile and make tweaks to get things working on the latest RHEL. Verify it passes testing and release it. Then they hope to not touch it again for a few years until the next RHEL comes along. Scary, huh? :-) Common examples, filesystems and storage drivers. There is no way that I see to provide a 100% guarantee, but if we do enough checks, we should be able to have a high degree of confidence the driver won't blow up. On the flip side, easy things in the kernel to do is: - provide the memory allocation (instead of having the driver staticly allocate) - provide functions to retrieve various internal data (instead of having the driver do direct referencing to deep internal elements) - cut down on some static inlines (and use accessory functions instead), etc. Those types of changes allow the OOT driver to be more ignorant of kernel changes and struct modifications. Look to Stanislav's responses for his ideas on new tooling. Thanks for helping! Cheers, Don > > > I am not sure what 'control vermagic' is, but it sounds like a string check, > > which won't protect against the boatload of backports we do to structs, > > enums, and functions. > > Basically vermagic is the string all modules and the kernel get, which > must match in order to load modules. If you have modversions disabled, > then vermagic includes the kernel version. If modversions is enabled, > then vermagic does not include the kernel version but the CRCs have to > also match. > > Controlling it explicitly is just a couple of lines where a distro can > control it (so they can update their kernel version without breaking). > It's not meant to solve everything, just the first one. > > > Currently we are exploring various ways to get smarter here. The genksyms > > tool has its limitations and handling kabi hacks in RHEL is getting > > tiresome. > > > > I think GregKH pointed to one such tool, libabigail? We are working on > > others too. > > > > > > Circling back to enabling MODVERSIONS in Fedora, that was to start the > > process of syncing Fedora with RHEL stuff in preparation for smarter tools. > > > > > > If you take away MODVERSIONS, that would put a damper in our work, but > > easily carried privately (much like MODSIGNING for 8 years until it went > > upstream :-) ). > > I don't think that's necessary. A feature requirement for a distro is just > as valid as any other user of upstream. I don't want to hinder any distro, > I'm just still not quite seeing the big picture of exactly what functionality > you need from the kernel. > > Thanks, > Nick
[toc] | [prev] | [next] | [standalone]
| From | Don Zickus <dzickus@redhat.com> |
|---|---|
| Date | 2016-12-01 17:00 +0100 |
| Subject | Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm |
| Message-ID | <sJBMu-5TK-19@gated-at.bofh.it> |
| In reply to | #1534201 |
On Thu, Dec 01, 2016 at 07:26:09AM -0800, Christoph Hellwig wrote: > On Thu, Dec 01, 2016 at 10:20:39AM -0500, Don Zickus wrote: > > > > - provide the memory allocation (instead of having the driver staticly > > allocate) > > - provide functions to retrieve various internal data (instead of having the > > driver do direct referencing to deep internal elements) > > - cut down on some static inlines (and use accessory functions instead), > > etc. > > > > Those types of changes allow the OOT driver to be more ignorant of kernel > > changes and struct modifications. > > All that is counter to what we really want to have: a well integrated > kernel that moves forward together so that we can see and improve the > whole situation. No need to make things worse just to help leeches. > Get your damn drivers upstream ASAP and let's stop this discussion.. I understand and won't disagree with you. :-) Unfortunately, there are various drivers that will never go upstream - paid storage drivers that provide bells and whistles on top of inbox driver - old drivers/fs that application has been relying on for a long time but company doesn't have resources to migrate to current technology. We have been trying over the years to do what we can to move customers in the right direction. It is just a slow process, sadly. Cheers, Don
[toc] | [prev] | [next] | [standalone]
| From | Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
|---|---|
| Date | 2016-12-01 17:40 +0100 |
| Subject | Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm |
| Message-ID | <sJC5Q-6gw-39@gated-at.bofh.it> |
| In reply to | #1534231 |
On Thu, Dec 01, 2016 at 10:40:59AM -0500, Don Zickus wrote: > Unfortunately, there are various drivers that will never go upstream > > - paid storage drivers that provide bells and whistles on top of inbox > driver That's because the developer doesn't want them upstream, that's their fault, nothing we can do about them. > - old drivers/fs that application has been relying on for a long time but > company doesn't have resources to migrate to current technology. That's what drivers/staging/ is for, I'll take anything that builds (and sometimes stuff that doesn't build) as long as people are actually using it. So send the stuff that is in this category on to me and that will reduce your burden a _lot_. thanks, greg k-h
[toc] | [prev] | [next] | [standalone]
| From | Don Zickus <dzickus@redhat.com> |
|---|---|
| Date | 2016-12-01 20:10 +0100 |
| Subject | Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm |
| Message-ID | <sJEAF-8gJ-5@gated-at.bofh.it> |
| In reply to | #1534272 |
On Thu, Dec 01, 2016 at 05:06:11PM +0100, Greg Kroah-Hartman wrote: > On Thu, Dec 01, 2016 at 10:40:59AM -0500, Don Zickus wrote: > > Unfortunately, there are various drivers that will never go upstream > > > > - paid storage drivers that provide bells and whistles on top of inbox > > driver > > That's because the developer doesn't want them upstream, that's their > fault, nothing we can do about them. > > > - old drivers/fs that application has been relying on for a long time but > > company doesn't have resources to migrate to current technology. > > That's what drivers/staging/ is for, I'll take anything that builds (and > sometimes stuff that doesn't build) as long as people are actually using > it. So send the stuff that is in this category on to me and that will > reduce your burden a _lot_. Hi Greg, I will forward this offer to the right folks and see who we can get to bite. :-) Thanks! Cheers, Don
[toc] | [prev] | [next] | [standalone]
| From | Christoph Hellwig <hch@infradead.org> |
|---|---|
| Date | 2016-12-01 17:10 +0100 |
| Subject | Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm |
| Message-ID | <sJBt7-5DE-21@gated-at.bofh.it> |
| In reply to | #1534201 |
On Thu, Dec 01, 2016 at 10:20:39AM -0500, Don Zickus wrote: > > - provide the memory allocation (instead of having the driver staticly > allocate) > - provide functions to retrieve various internal data (instead of having the > driver do direct referencing to deep internal elements) > - cut down on some static inlines (and use accessory functions instead), > etc. > > Those types of changes allow the OOT driver to be more ignorant of kernel > changes and struct modifications. All that is counter to what we really want to have: a well integrated kernel that moves forward together so that we can see and improve the whole situation. No need to make things worse just to help leeches. Get your damn drivers upstream ASAP and let's stop this discussion..
[toc] | [prev] | [next] | [standalone]
| From | Nicholas Piggin <npiggin@gmail.com> |
|---|---|
| Date | 2016-12-09 05:10 +0100 |
| Subject | Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm |
| Message-ID | <sMkvQ-6CI-7@gated-at.bofh.it> |
| In reply to | #1534201 |
On Thu, 1 Dec 2016 10:20:39 -0500 Don Zickus <dzickus@redhat.com> wrote: > On Thu, Dec 01, 2016 at 03:32:15PM +1100, Nicholas Piggin wrote: > > > Anyway, MODVERSIONS is our way of protecting our kabi for the last 10 years. > > > It isn't perfect and we have fixed the genksyms tool over the years, but so > > > far it mostly works fine. > > > > Okay. It would be good to get all the distros in on this. > > > > What I want to do is work out exactly what it is that modversions is > > giving you. > > > > We know it's fairly nasty code to maintain and it does not detect ABI > > changes very well. But it's not such a burden that we can't maintain > > it if there are good reasons to keep it. > > Hi Nick, > > I won't disagree with you there. :-) Sorry for the late reply, I was moving house and got side tracked. > 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. It's heavy handed in that it is quite complex in the kernel build system, but it is also light handed in that it does not do a very good job. I would say the degree of confidence is not very high. People have told me modversions follows pointers to objects in its calculation, but I have not seen that to be the case. Even if you did have that, it can not replace a code review for semantics of data and code. > 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). > > We have plenty of customers with 10 year old drivers, where the expertise > has long left the company. The engineers still around, recompile and make > tweaks to get things working on the latest RHEL. Verify it passes testing > and release it. Then they hope to not touch it again for a few years until > the next RHEL comes along. > > Scary, huh? :-) Oh yeah my aim here is not to make distro or out of tree module vendors life harder, actually the opposite. If it turns out modversions really is the best approach, I'm not in a position to complain about its complexity because we have Suse and Redhat people maintaining the build and module systems :) I just want to see if we can do things better. Thanks, Nick
[toc] | [prev] | [next] | [standalone]
| From | Stanislav Kozina <skozina@redhat.com> |
|---|---|
| Date | 2016-12-09 09:20 +0100 |
| Subject | Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm |
| Message-ID | <sMog1-xv-5@gated-at.bofh.it> |
| In reply to | #1539058 |
>> 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. Regards, -Stanislav
[toc] | [prev] | [next] | [standalone]
| From | Nicholas Piggin <npiggin@gmail.com> |
|---|---|
| Date | 2016-12-09 09:40 +0100 |
| Subject | Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm |
| Message-ID | <sMozn-T1-23@gated-at.bofh.it> |
| In reply to | #1539175 |
On Fri, 9 Dec 2016 08:55:51 +0100 Stanislav Kozina <skozina@redhat.com> wrote: > >> 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)? Thanks, Nick
[toc] | [prev] | [next] | [standalone]
| From | Stanislav Kozina <skozina@redhat.com> |
|---|---|
| Date | 2016-12-09 16:00 +0100 |
| Subject | Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm |
| Message-ID | <sMuv7-4qd-3@gated-at.bofh.it> |
| In reply to | #1539188 |
>>>> 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)? 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). Thanks, -Stanislav
[toc] | [prev] | [next] | [standalone]
| From | Nicholas Piggin <npiggin@gmail.com> |
|---|---|
| Date | 2016-12-09 17:20 +0100 |
| Subject | Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm |
| Message-ID | <sMvKy-55a-13@gated-at.bofh.it> |
| In reply to | #1539423 |
On Fri, 9 Dec 2016 15:36:04 +0100 Stanislav Kozina <skozina@redhat.com> wrote: > >>>> 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)? > > 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? Thanks, Nick
[toc] | [prev] | [next] | [standalone]
| From | Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
|---|---|
| Date | 2016-12-09 17:30 +0100 |
| Subject | Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm |
| Message-ID | <sMvUe-5nQ-1@gated-at.bofh.it> |
| In reply to | #1539477 |
On Sat, Dec 10, 2016 at 01:56:53AM +1000, Nicholas Piggin wrote: > On Fri, 9 Dec 2016 15:36:04 +0100 > Stanislav Kozina <skozina@redhat.com> wrote: > > > >>>> 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)? > > > > 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? 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 :) thanks, greg k-h
[toc] | [prev] | [next] | [standalone]
| From | Stanislav Kozina <skozina@redhat.com> |
|---|---|
| Date | 2016-12-12 11:10 +0100 |
| Subject | Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm |
| Message-ID | <sNvp8-2Ek-3@gated-at.bofh.it> |
| In reply to | #1539531 |
>>>> 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). >> 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. Warm Regards, -Stanislav
[toc] | [prev] | [next] | [standalone]
| From | Nicholas Piggin <npiggin@gmail.com> |
|---|---|
| Date | 2016-12-13 08:50 +0100 |
| Subject | Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm |
| Message-ID | <sNPHb-6JH-11@gated-at.bofh.it> |
| In reply to | #1540227 |
On Mon, 12 Dec 2016 10:48:47 +0100 Stanislav Kozina <skozina@redhat.com> wrote: > >>>> 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). > >> 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. That kind of name clash should hardly be an issue, in comparison with the care it requires to merge fixes on top of a backport which has already changed ABI. It tends to be trivially fixable just by search/replace in the patchfile before applying, if nothing else. But you probably *want* to be flagged on those things and merge by hand anyway. Backporting alone I don't think can justify symbol versioning, but I'd like to hear from distro maintainers if any disagree. Thanks, Nick
[toc] | [prev] | [next] | [standalone]
| From | Hannes Frederic Sowa <hannes@redhat.com> |
|---|---|
| Date | 2016-12-14 15:40 +0100 |
| Subject | Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm |
| Message-ID | <sOizw-7tt-3@gated-at.bofh.it> |
| In reply to | #1539531 |
On 09.12.2016 17:03, Greg Kroah-Hartman wrote: > On Sat, Dec 10, 2016 at 01:56:53AM +1000, Nicholas Piggin wrote: >> On Fri, 9 Dec 2016 15:36:04 +0100 >> Stanislav Kozina <skozina@redhat.com> wrote: >> >>>>>>> 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)? >>> >>> 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? > > 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. Thanks, Hannes (who is still impressed by the genksyms tools)
[toc] | [prev] | [next] | [standalone]
| From | Nicholas Piggin <npiggin@gmail.com> |
|---|---|
| Date | 2016-12-15 03:30 +0100 |
| Subject | Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm |
| Message-ID | <sOtEB-8g3-3@gated-at.bofh.it> |
| In reply to | #1541948 |
On Wed, 14 Dec 2016 15:04:36 +0100 Hannes Frederic Sowa <hannes@redhat.com> wrote: > On 09.12.2016 17:03, Greg Kroah-Hartman wrote: > > On Sat, Dec 10, 2016 at 01:56:53AM +1000, Nicholas Piggin wrote: > >> On Fri, 9 Dec 2016 15:36:04 +0100 > >> Stanislav Kozina <skozina@redhat.com> wrote: > >> > >>>>>>> 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)? > >>> > >>> 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? > > > > 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. 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? > > 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. Thanks, Nick
[toc] | [prev] | [next] | [standalone]
| From | Hannes Frederic Sowa <hannes@redhat.com> |
|---|---|
| Date | 2016-12-15 12:40 +0100 |
| Subject | Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm |
| Message-ID | <sOCeR-55V-9@gated-at.bofh.it> |
| In reply to | #1542444 |
On 15.12.2016 03:06, Nicholas Piggin wrote: > On Wed, 14 Dec 2016 15:04:36 +0100 > Hannes Frederic Sowa <hannes@redhat.com> wrote: > >> On 09.12.2016 17:03, Greg Kroah-Hartman wrote: >>> On Sat, Dec 10, 2016 at 01:56:53AM +1000, Nicholas Piggin wrote: >>>> On Fri, 9 Dec 2016 15:36:04 +0100 >>>> Stanislav Kozina <skozina@redhat.com> wrote: >>>> >>>>>>>>> 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)? >>>>> >>>>> 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? >>> >>> 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. > > 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. >> 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. Bye, Hannes
[toc] | [prev] | [next] | [standalone]
| From | Nicholas Piggin <npiggin@gmail.com> |
|---|---|
| Date | 2016-12-15 13:30 +0100 |
| Subject | Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm |
| Message-ID | <sOD1g-5AK-29@gated-at.bofh.it> |
| In reply to | #1542651 |
On Thu, 15 Dec 2016 12:19:02 +0100 Hannes Frederic Sowa <hannes@redhat.com> wrote: > On 15.12.2016 03:06, Nicholas Piggin wrote: > > On Wed, 14 Dec 2016 15:04:36 +0100 > > Hannes Frederic Sowa <hannes@redhat.com> wrote: > > > >> On 09.12.2016 17:03, Greg Kroah-Hartman wrote: > >>> On Sat, Dec 10, 2016 at 01:56:53AM +1000, Nicholas Piggin wrote: > >>>> On Fri, 9 Dec 2016 15:36:04 +0100 > >>>> Stanislav Kozina <skozina@redhat.com> wrote: > >>>> > >>>>>>>>> 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)? > >>>>> > >>>>> 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? > >>> > >>> 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. > > > > 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...). 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. > > 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. > > >> 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. 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. > 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. Thanks, Nick
[toc] | [prev] | [next] | [standalone]
| From | Hannes Frederic Sowa <hannes@redhat.com> |
|---|---|
| Date | 2016-12-15 14:40 +0100 |
| Subject | Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm |
| Message-ID | <sOE6Z-6cE-11@gated-at.bofh.it> |
| In reply to | #1542703 |
On 15.12.2016 13:03, Nicholas Piggin wrote: > On Thu, 15 Dec 2016 12:19:02 +0100 > Hannes Frederic Sowa <hannes@redhat.com> wrote: > >> On 15.12.2016 03:06, Nicholas Piggin wrote: >>> On Wed, 14 Dec 2016 15:04:36 +0100 >>> Hannes Frederic Sowa <hannes@redhat.com> wrote: >>> >>>> On 09.12.2016 17:03, Greg Kroah-Hartman wrote: >>>>> On Sat, Dec 10, 2016 at 01:56:53AM +1000, Nicholas Piggin wrote: >>>>>> On Fri, 9 Dec 2016 15:36:04 +0100 >>>>>> Stanislav Kozina <skozina@redhat.com> wrote: >>>>>> >>>>>>>>>>> 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)? >>>>>>> >>>>>>> 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? >>>>> >>>>> 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. >>> >>> 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...). > > 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. >> 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. >> >>>> 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. > > 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. >> 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. Agreed! Thanks also, Hannes
[toc] | [prev] | [next] | [standalone]
| From | Nicholas Piggin <npiggin@gmail.com> |
|---|---|
| Date | 2016-12-15 15:40 +0100 |
| Subject | Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm |
| Message-ID | <sOF33-6KZ-11@gated-at.bofh.it> |
| In reply to | #1542736 |
On Thu, 15 Dec 2016 14:15:31 +0100 Hannes Frederic Sowa <hannes@redhat.com> wrote: > On 15.12.2016 13:03, Nicholas Piggin wrote: > > On Thu, 15 Dec 2016 12:19:02 +0100 > > Hannes Frederic Sowa <hannes@redhat.com> wrote: > > > >> On 15.12.2016 03:06, Nicholas Piggin wrote: > >>> On Wed, 14 Dec 2016 15:04:36 +0100 > >>> Hannes Frederic Sowa <hannes@redhat.com> wrote: > >>> > >>>> On 09.12.2016 17:03, Greg Kroah-Hartman wrote: > >>>>> On Sat, Dec 10, 2016 at 01:56:53AM +1000, Nicholas Piggin wrote: > >>>>>> On Fri, 9 Dec 2016 15:36:04 +0100 > >>>>>> Stanislav Kozina <skozina@redhat.com> wrote: > >>>>>> > >>>>>>>>>>> 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)? > >>>>>>> > >>>>>>> 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? > >>>>> > >>>>> 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. > >>> > >>> 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...). > > > > 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. 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? > >> 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. 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. > >> 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. > > Agreed! Thanks, Nick
[toc] | [prev] | [next] | [standalone]
Page 3 of 5 — ← Prev page 1 2 [3] 4 5 Next page →
Back to top | Article view | linux.kernel
csiph-web