Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756594AbcK2JOq (ORCPT ); Tue, 29 Nov 2016 04:14:46 -0500 Received: from mx2.suse.de ([195.135.220.15]:48439 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756486AbcK2JOR (ORCPT ); Tue, 29 Nov 2016 04:14:17 -0500 Subject: Re: [PATCH] x86/kbuild: enable modversions for symbols exported from asm To: Nicholas Piggin , Ben Hutchings References: <1480382148.16599.61.camel@decadent.org.uk> <20161129133130.164b4e2d@roar.ozlabs.ibm.com> Cc: Linus Torvalds , linux-arch@vger.kernel.org, linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org, Greg Kroah-Hartman , Arnd Bergmann , Ingo Molnar , Adam Borowski , Debian kernel maintainers From: Michal Marek X-Enigmail-Draft-Status: N1110 Message-ID: <89fa3c90-dad3-b4f4-a343-6264f2d7b220@suse.com> Date: Tue, 29 Nov 2016 10:14:08 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <20161129133130.164b4e2d@roar.ozlabs.ibm.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3997 Lines: 97 Dne 29.11.2016 v 03:31 Nicholas Piggin napsal(a): > On Tue, 29 Nov 2016 01:15:48 +0000 > Ben Hutchings wrote: > >> [I've had to guess at the cc list for this, because we no longer have >> mail archives that preserve them.] > > You got it about right. > >> On Fri, 2016-11-25 at 10:01 -0800, Linus Torvalds wrote: >>> On Thu, Nov 24, 2016 at 4:40 PM, Nicholas Piggin wrote: >>>>> >>>>> Yes, manual "marking" is never going to be a viable solution. >>>> >>>> I guess it really depends on how exactly you want to use it. For distros >>>> that do stable ABI but rarely may have to break something for security >>>> reasons, it should work and give exact control. >> >> This is roughly how Debian handles the kernel module ABI during a >> stable release. >> >>> No. Because nobody else will care, so unless it's like a single symbol >>> or something, it will just be a maintenance nightmare. >> >> I agree with this. We can explicitly "version" individual symbols >> anyway by doing something like: >> >> -int foo(void); >> +#define foo foo_2 >> +int foo_2(int); > > Yeah... Benefit being it's very simple and everybody can see exactly > what it does and knows how it will work. > >> >>>> What else do people *actually* use it for? Preventing mismatched modules >>>> when .git version is not attached and release version of the kernel has >>>> not been bumped. Is that it? >>> >>> It used to be very useful for avoiding loading stale modules and then >>> wasting days on debugging something that wasn't the case when you had >>> forgotten to do "make modules_install". Change some subtle internal >>> ABI issue (add/remove a parameter, whatever) and it would really help. >>> >>> These days, for me, LOCALVERSION_AUTO and module signing are what I >>> personally tend to use. >>> >>> The modversions stuff may just be too painful to bother with. Very few >>> people probably use it, and the ones that do likely don't have any >>> overriding reason why. >> [...] >> >> Debian has some strong reasons: I guess many distros have similar reasons. >> 1. Changing the release string requires any out-of-tree modules to be >> upgraded (at least rebuilt) on end-user systems. So we try to avoid >> doing that during the lifetime of a stable release, i.e. we don't let >> the release string change. Also, the release string is reflected in >> package names (e.g. linux-image-4.8.0-1-amd64), and introducing new >> package names requires manual approval by the Debian archive team. > > This is something I've noticed. Would it be better if the module loader > ignores the kernel version and instead used some internal ABI version > string to check against? Otherwise (AFAICT) you always have 4.8.0 versions > despite being 4.8.7 kernel, and you can't upgrade a point release without > overwriting your old kernel and modules. The thing is - to maintain an ABI version string, you need some level of certainty that two given ABIs are really interchangeable. Which means you need to check whether the symbols _and_ types exposed are unchanged. Which is a thing that genksyms, the tool behind CONFIG_MODVERSIONS, does quite well. So yes, you could do a testbuild with CONFIG_MODVERSIONS=y and a production build with some global ABI string, but what's the point then. > It would be nice to get upstream to the point where 4.9 modversions > works if you just patch out depends BROKEN. That would require reverting > a few more of Al's arch patches. > > Then in 4.10 we can re-add all those arch patches (which are less > controversial without the asm-prototypes.h workaround), and implement a > simple stable ABI version string check, and then in 4.11 we can remove > modversions. I'd rather change the kconfig to depends on BROKEN || and eventuallly remove the dependency again. PPC has the header already, so it can be added right away. I do not know why the x86 patch has not been merged yet. Michal