| Message ID | 20210206035033.2036180-2-sashal@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] Revert "kbuild: give the SUBLEVEL more room in KERNEL_VERSION" | expand |
On Fri, Feb 05, 2021 at 10:50:32PM -0500, Sasha Levin wrote: > Right now if SUBLEVEL becomes larger than 255 it will overflow into the > territory of PATCHLEVEL, causing havoc in userspace that tests for > specific kernel version. > > While userspace code tests for MAJOR and PATCHLEVEL, it doesn't test > SUBLEVEL at any point as ABI changes don't happen in the context of > stable tree. > > Thus, to avoid overflows, simply clamp SUBLEVEL to it's maximum value in > the context of LINUX_VERSION_CODE. This does not affect "make > kernelversion" and such. > > Signed-off-by: Sasha Levin <sashal@kernel.org> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
On Sat, Feb 6, 2021 at 12:50 PM Sasha Levin <sashal@kernel.org> wrote: > > Right now if SUBLEVEL becomes larger than 255 it will overflow into the > territory of PATCHLEVEL, causing havoc in userspace that tests for > specific kernel version. > > While userspace code tests for MAJOR and PATCHLEVEL, it doesn't test > SUBLEVEL at any point as ABI changes don't happen in the context of > stable tree. > > Thus, to avoid overflows, simply clamp SUBLEVEL to it's maximum value in > the context of LINUX_VERSION_CODE. This does not affect "make > kernelversion" and such. > > Signed-off-by: Sasha Levin <sashal@kernel.org> I applied 2/3 to linux-kbuild. Please resend only 3/3. Thanks. > --- > Makefile | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/Makefile b/Makefile > index 49ac1b7fe8e99..157be50c691e5 100644 > --- a/Makefile > +++ b/Makefile > @@ -1258,9 +1258,15 @@ define filechk_utsrelease.h > endef > > define filechk_version.h > - echo \#define LINUX_VERSION_CODE $(shell \ > - expr $(VERSION) \* 65536 + 0$(PATCHLEVEL) \* 256 + 0$(SUBLEVEL)); \ > - echo '#define KERNEL_VERSION(a,b,c) (((a) << 16) + ((b) << 8) + (c))' > + if [ $(SUBLEVEL) -gt 255 ]; then \ > + echo \#define LINUX_VERSION_CODE $(shell \ > + expr $(VERSION) \* 65536 + 0$(PATCHLEVEL) \* 256 + 255); \ > + else \ > + echo \#define LINUX_VERSION_CODE $(shell \ > + expr $(VERSION) \* 65536 + 0$(PATCHLEVEL) \* 256 + $(SUBLEVEL)); \ > + fi; \ > + echo '#define KERNEL_VERSION(a,b,c) (((a) << 16) + ((b) << 8) + \ > + ((c) > 255 ? 255 : (c)))' > endef > > $(version_h): FORCE > -- > 2.27.0 >
From: Sasha Levin > Sent: 06 February 2021 03:51 > > Right now if SUBLEVEL becomes larger than 255 it will overflow into the > territory of PATCHLEVEL, causing havoc in userspace that tests for > specific kernel version. > > While userspace code tests for MAJOR and PATCHLEVEL, it doesn't test > SUBLEVEL at any point as ABI changes don't happen in the context of > stable tree. > > Thus, to avoid overflows, simply clamp SUBLEVEL to it's maximum value in > the context of LINUX_VERSION_CODE. This does not affect "make > kernelversion" and such. > > Signed-off-by: Sasha Levin <sashal@kernel.org> > --- > Makefile | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/Makefile b/Makefile > index 49ac1b7fe8e99..157be50c691e5 100644 > --- a/Makefile > +++ b/Makefile > @@ -1258,9 +1258,15 @@ define filechk_utsrelease.h > endef > > define filechk_version.h > - echo \#define LINUX_VERSION_CODE $(shell \ > - expr $(VERSION) \* 65536 + 0$(PATCHLEVEL) \* 256 + 0$(SUBLEVEL)); \ > - echo '#define KERNEL_VERSION(a,b,c) (((a) << 16) + ((b) << 8) + (c))' > + if [ $(SUBLEVEL) -gt 255 ]; then \ > + echo \#define LINUX_VERSION_CODE $(shell \ > + expr $(VERSION) \* 65536 + 0$(PATCHLEVEL) \* 256 + 255); \ > + else \ > + echo \#define LINUX_VERSION_CODE $(shell \ > + expr $(VERSION) \* 65536 + 0$(PATCHLEVEL) \* 256 + $(SUBLEVEL)); \ > + fi; \ > + echo '#define KERNEL_VERSION(a,b,c) (((a) << 16) + ((b) << 8) + \ > + ((c) > 255 ? 255 : (c)))' > endef Why not use KERNEL_VERSION to define LINUX_VERSION_CODE ? Basically just: echo '#define LINUX_VERSION_CODE KERNEL_VERSION($(VERSION), $(PATCHLEVEL)+0, $(SUBLEVEL)+0)' If PATCHLEVEL and SUBLEVEL are guaranteed to be non-empty the +0 can be removed. The patch assumes they are non-empty, the original pre-prended 0 to stop syntax error for empty version strings. Note that the expr version will process 08 and 09. gcc will treat them as octal, and may error them. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
On Mon, Feb 08, 2021 at 01:48:06PM +0000, David Laight wrote: > From: Sasha Levin > > Sent: 06 February 2021 03:51 > > > > Right now if SUBLEVEL becomes larger than 255 it will overflow into the > > territory of PATCHLEVEL, causing havoc in userspace that tests for > > specific kernel version. > > > > While userspace code tests for MAJOR and PATCHLEVEL, it doesn't test > > SUBLEVEL at any point as ABI changes don't happen in the context of > > stable tree. > > > > Thus, to avoid overflows, simply clamp SUBLEVEL to it's maximum value in > > the context of LINUX_VERSION_CODE. This does not affect "make > > kernelversion" and such. > > > > Signed-off-by: Sasha Levin <sashal@kernel.org> > > --- > > Makefile | 12 +++++++++--- > > 1 file changed, 9 insertions(+), 3 deletions(-) > > > > diff --git a/Makefile b/Makefile > > index 49ac1b7fe8e99..157be50c691e5 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -1258,9 +1258,15 @@ define filechk_utsrelease.h > > endef > > > > define filechk_version.h > > - echo \#define LINUX_VERSION_CODE $(shell \ > > - expr $(VERSION) \* 65536 + 0$(PATCHLEVEL) \* 256 + 0$(SUBLEVEL)); \ > > - echo '#define KERNEL_VERSION(a,b,c) (((a) << 16) + ((b) << 8) + (c))' > > + if [ $(SUBLEVEL) -gt 255 ]; then \ > > + echo \#define LINUX_VERSION_CODE $(shell \ > > + expr $(VERSION) \* 65536 + 0$(PATCHLEVEL) \* 256 + 255); \ > > + else \ > > + echo \#define LINUX_VERSION_CODE $(shell \ > > + expr $(VERSION) \* 65536 + 0$(PATCHLEVEL) \* 256 + $(SUBLEVEL)); \ > > + fi; \ > > + echo '#define KERNEL_VERSION(a,b,c) (((a) << 16) + ((b) << 8) + \ > > + ((c) > 255 ? 255 : (c)))' > > endef > > Why not use KERNEL_VERSION to define LINUX_VERSION_CODE ? > Basically just: > echo '#define LINUX_VERSION_CODE KERNEL_VERSION($(VERSION), $(PATCHLEVEL)+0, $(SUBLEVEL)+0)' Because we are "clamping" LINUX_VERSION_CODE() at a x.y.255, while KERNEL_VERSION() continues on with the "real" minor number. thanks, greg k-h
From: gregkh@linuxfoundation.org > Sent: 08 February 2021 14:09 > > On Mon, Feb 08, 2021 at 01:48:06PM +0000, David Laight wrote: > > From: Sasha Levin > > > Sent: 06 February 2021 03:51 > > > > > > Right now if SUBLEVEL becomes larger than 255 it will overflow into the > > > territory of PATCHLEVEL, causing havoc in userspace that tests for > > > specific kernel version. > > > > > > While userspace code tests for MAJOR and PATCHLEVEL, it doesn't test > > > SUBLEVEL at any point as ABI changes don't happen in the context of > > > stable tree. > > > > > > Thus, to avoid overflows, simply clamp SUBLEVEL to it's maximum value in > > > the context of LINUX_VERSION_CODE. This does not affect "make > > > kernelversion" and such. > > > > > > Signed-off-by: Sasha Levin <sashal@kernel.org> > > > --- > > > Makefile | 12 +++++++++--- > > > 1 file changed, 9 insertions(+), 3 deletions(-) > > > > > > diff --git a/Makefile b/Makefile > > > index 49ac1b7fe8e99..157be50c691e5 100644 > > > --- a/Makefile > > > +++ b/Makefile > > > @@ -1258,9 +1258,15 @@ define filechk_utsrelease.h > > > endef > > > > > > define filechk_version.h > > > - echo \#define LINUX_VERSION_CODE $(shell \ > > > - expr $(VERSION) \* 65536 + 0$(PATCHLEVEL) \* 256 + 0$(SUBLEVEL)); \ > > > - echo '#define KERNEL_VERSION(a,b,c) (((a) << 16) + ((b) << 8) + (c))' > > > + if [ $(SUBLEVEL) -gt 255 ]; then \ > > > + echo \#define LINUX_VERSION_CODE $(shell \ > > > + expr $(VERSION) \* 65536 + 0$(PATCHLEVEL) \* 256 + 255); \ > > > + else \ > > > + echo \#define LINUX_VERSION_CODE $(shell \ > > > + expr $(VERSION) \* 65536 + 0$(PATCHLEVEL) \* 256 + $(SUBLEVEL)); \ > > > + fi; \ > > > + echo '#define KERNEL_VERSION(a,b,c) (((a) << 16) + ((b) << 8) + \ > > > + ((c) > 255 ? 255 : (c)))' > > > endef > > > > Why not use KERNEL_VERSION to define LINUX_VERSION_CODE ? > > Basically just: > > echo '#define LINUX_VERSION_CODE KERNEL_VERSION($(VERSION), $(PATCHLEVEL)+0, $(SUBLEVEL)+0)' > > Because we are "clamping" LINUX_VERSION_CODE() at a x.y.255, while > KERNEL_VERSION() continues on with the "real" minor number. That particular patch (which I think Sasha applied) clamps both. I know you (personally) don't care about OOT mudules, but a lot of people do. They will contain comparisons of KERNEL_VERSION against LINUX_VERSION_CODE. They almost certainly don't care about the SUBLEVEL, but using different encodings will break things. The only real saving grace is that OOT modules tend to get built against distro kernels which are likely to be 4.9.4-123456 so not hit whatever broken mapping you use for LTS kernels. Another strange set of '#ifs' for new kernels is just part of the job. It keeps me in work. For a comparison the same driver object will load into windows 7 (and maybe even vista) and the current windows 10 kernel. (But getting a windows driver signed is hard work.) David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
On Mon, Feb 8, 2021 at 10:48 PM David Laight <David.Laight@aculab.com> wrote: > > From: Sasha Levin > > Sent: 06 February 2021 03:51 > > > > Right now if SUBLEVEL becomes larger than 255 it will overflow into the > > territory of PATCHLEVEL, causing havoc in userspace that tests for > > specific kernel version. > > > > While userspace code tests for MAJOR and PATCHLEVEL, it doesn't test > > SUBLEVEL at any point as ABI changes don't happen in the context of > > stable tree. > > > > Thus, to avoid overflows, simply clamp SUBLEVEL to it's maximum value in > > the context of LINUX_VERSION_CODE. This does not affect "make > > kernelversion" and such. > > > > Signed-off-by: Sasha Levin <sashal@kernel.org> > > --- > > Makefile | 12 +++++++++--- > > 1 file changed, 9 insertions(+), 3 deletions(-) > > > > diff --git a/Makefile b/Makefile > > index 49ac1b7fe8e99..157be50c691e5 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -1258,9 +1258,15 @@ define filechk_utsrelease.h > > endef > > > > define filechk_version.h > > - echo \#define LINUX_VERSION_CODE $(shell \ > > - expr $(VERSION) \* 65536 + 0$(PATCHLEVEL) \* 256 + 0$(SUBLEVEL)); \ > > - echo '#define KERNEL_VERSION(a,b,c) (((a) << 16) + ((b) << 8) + (c))' > > + if [ $(SUBLEVEL) -gt 255 ]; then \ > > + echo \#define LINUX_VERSION_CODE $(shell \ > > + expr $(VERSION) \* 65536 + 0$(PATCHLEVEL) \* 256 + 255); \ > > + else \ > > + echo \#define LINUX_VERSION_CODE $(shell \ > > + expr $(VERSION) \* 65536 + 0$(PATCHLEVEL) \* 256 + $(SUBLEVEL)); \ > > + fi; \ > > + echo '#define KERNEL_VERSION(a,b,c) (((a) << 16) + ((b) << 8) + \ > > + ((c) > 255 ? 255 : (c)))' > > endef > > Why not use KERNEL_VERSION to define LINUX_VERSION_CODE ? > Basically just: > echo '#define LINUX_VERSION_CODE KERNEL_VERSION($(VERSION), $(PATCHLEVEL)+0, $(SUBLEVEL)+0)' It was not possible to macrofy LINUX_VERSION_CODE. (build error when CONFIG_KALLSYMS is disabled) Presumably, this restriction will go away with the following commit in linux-next. commit e06af0b2ba02fc0cc2219a14c4c04ff0296a6f9f Author: Masahiro Yamada <masahiroy@kernel.org> Date: Thu Jan 28 18:42:30 2021 +1100 init/version.c: remove Version_<LINUX_VERSION_CODE> symbol My plan is to refactor LINUX_VERSION_CODE in the next development cycle. > If PATCHLEVEL and SUBLEVEL are guaranteed to be non-empty the +0 > can be removed. > The patch assumes they are non-empty, the original pre-prended 0 > to stop syntax error for empty version strings. > > Note that the expr version will process 08 and 09. > gcc will treat them as octal, and may error them. > > David > > - > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK > Registration No: 1397386 (Wales) > -- Best Regards Masahiro Yamada
diff --git a/Makefile b/Makefile index 49ac1b7fe8e99..157be50c691e5 100644 --- a/Makefile +++ b/Makefile @@ -1258,9 +1258,15 @@ define filechk_utsrelease.h endef define filechk_version.h - echo \#define LINUX_VERSION_CODE $(shell \ - expr $(VERSION) \* 65536 + 0$(PATCHLEVEL) \* 256 + 0$(SUBLEVEL)); \ - echo '#define KERNEL_VERSION(a,b,c) (((a) << 16) + ((b) << 8) + (c))' + if [ $(SUBLEVEL) -gt 255 ]; then \ + echo \#define LINUX_VERSION_CODE $(shell \ + expr $(VERSION) \* 65536 + 0$(PATCHLEVEL) \* 256 + 255); \ + else \ + echo \#define LINUX_VERSION_CODE $(shell \ + expr $(VERSION) \* 65536 + 0$(PATCHLEVEL) \* 256 + $(SUBLEVEL)); \ + fi; \ + echo '#define KERNEL_VERSION(a,b,c) (((a) << 16) + ((b) << 8) + \ + ((c) > 255 ? 255 : (c)))' endef $(version_h): FORCE
Right now if SUBLEVEL becomes larger than 255 it will overflow into the territory of PATCHLEVEL, causing havoc in userspace that tests for specific kernel version. While userspace code tests for MAJOR and PATCHLEVEL, it doesn't test SUBLEVEL at any point as ABI changes don't happen in the context of stable tree. Thus, to avoid overflows, simply clamp SUBLEVEL to it's maximum value in the context of LINUX_VERSION_CODE. This does not affect "make kernelversion" and such. Signed-off-by: Sasha Levin <sashal@kernel.org> --- Makefile | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)