linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] Revert "kbuild: give the SUBLEVEL more room in KERNEL_VERSION"
@ 2021-02-06  3:50 Sasha Levin
  2021-02-06  3:50 ` [PATCH 2/3] kbuild: clamp SUBLEVEL to 255 Sasha Levin
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Sasha Levin @ 2021-02-06  3:50 UTC (permalink / raw)
  To: masahiroy, michal.lkml
  Cc: linux-kbuild, linux-kernel, torvalds, gregkh, Sasha Levin

This reverts commit 537896fabed11f8d9788886d1aacdb977213c7b3.

This turns out to be a bad idea: userspace has coded the structure of
KERNEL_VERSION on it's own and assumes the 2-1-1 byte split, making it
userspace ABI we can't break.

The reverted patch didn't make it past linux-next, so no userspace was
hurt in the process.

Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 Makefile                                       | 7 ++-----
 drivers/net/ethernet/mellanox/mlx5/core/main.c | 4 ++--
 drivers/usb/core/hcd.c                         | 4 ++--
 drivers/usb/gadget/udc/aspeed-vhub/hub.c       | 4 ++--
 include/linux/usb/composite.h                  | 4 ++--
 kernel/sys.c                                   | 2 +-
 tools/perf/tests/bpf-script-example.c          | 2 +-
 tools/perf/tests/bpf-script-test-kbuild.c      | 2 +-
 tools/perf/tests/bpf-script-test-prologue.c    | 2 +-
 9 files changed, 14 insertions(+), 17 deletions(-)

diff --git a/Makefile b/Makefile
index 28019532e55ac..49ac1b7fe8e99 100644
--- a/Makefile
+++ b/Makefile
@@ -1259,11 +1259,8 @@ endef
 
 define filechk_version.h
 	echo \#define LINUX_VERSION_CODE $(shell                         \
-	expr $(VERSION) \* 16777216 + 0$(PATCHLEVEL) \* 65536 + 0$(SUBLEVEL)); \
-	echo \#define LINUX_VERSION_MAJOR $(VERSION); \
-	echo \#define LINUX_VERSION_PATCHLEVEL $(PATCHLEVEL); \
-	echo \#define LINUX_VERSION_SUBLEVEL $(SUBLEVEL); \
-	echo '#define KERNEL_VERSION(a,b,c) (((a) << 24) + ((b) << 16) + (c))'
+	expr $(VERSION) \* 65536 + 0$(PATCHLEVEL) \* 256 + 0$(SUBLEVEL)); \
+	echo '#define KERNEL_VERSION(a,b,c) (((a) << 16) + ((b) << 8) + (c))'
 endef
 
 $(version_h): FORCE
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
index 989f15d9aa7d4..e4c9627485aa5 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
@@ -237,8 +237,8 @@ static void mlx5_set_driver_version(struct mlx5_core_dev *dev)
 	remaining_size = max_t(int, 0, driver_ver_sz - strlen(string));
 
 	snprintf(string + strlen(string), remaining_size, "%u.%u.%u",
-		(u8)(LINUX_VERSION_MAJOR), (u8)(LINUX_VERSION_PATCHLEVEL),
-		(u16)(LINUX_VERSION_SUBLEVEL));
+		 (u8)((LINUX_VERSION_CODE >> 16) & 0xff), (u8)((LINUX_VERSION_CODE >> 8) & 0xff),
+		 (u16)(LINUX_VERSION_CODE & 0xffff));
 
 	/*Send the command*/
 	MLX5_SET(set_driver_version_in, in, opcode,
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 3f0381344221e..ad5a0f405a75c 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -111,8 +111,8 @@ DECLARE_WAIT_QUEUE_HEAD(usb_kill_urb_queue);
  */
 
 /*-------------------------------------------------------------------------*/
-#define KERNEL_REL	bin2bcd(LINUX_VERSION_MAJOR)
-#define KERNEL_VER	bin2bcd(LINUX_VERSION_PATCHLEVEL)
+#define KERNEL_REL	bin2bcd(((LINUX_VERSION_CODE >> 16) & 0x0ff))
+#define KERNEL_VER	bin2bcd(((LINUX_VERSION_CODE >> 8) & 0x0ff))
 
 /* usb 3.1 root hub device descriptor */
 static const u8 usb31_rh_dev_descriptor[18] = {
diff --git a/drivers/usb/gadget/udc/aspeed-vhub/hub.c b/drivers/usb/gadget/udc/aspeed-vhub/hub.c
index 5c7dea5e0ff16..bfd8e77788e29 100644
--- a/drivers/usb/gadget/udc/aspeed-vhub/hub.c
+++ b/drivers/usb/gadget/udc/aspeed-vhub/hub.c
@@ -46,8 +46,8 @@
  *    - Make vid/did overridable
  *    - make it look like usb1 if usb1 mode forced
  */
-#define KERNEL_REL	bin2bcd(LINUX_VERSION_MAJOR)
-#define KERNEL_VER	bin2bcd(LINUX_VERSION_PATCHLEVEL)
+#define KERNEL_REL	bin2bcd(((LINUX_VERSION_CODE >> 16) & 0x0ff))
+#define KERNEL_VER	bin2bcd(((LINUX_VERSION_CODE >> 8) & 0x0ff))
 
 enum {
 	AST_VHUB_STR_INDEX_MAX = 4,
diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
index c71150f2c6390..5646dad886e61 100644
--- a/include/linux/usb/composite.h
+++ b/include/linux/usb/composite.h
@@ -575,8 +575,8 @@ static inline u16 get_default_bcdDevice(void)
 {
 	u16 bcdDevice;
 
-	bcdDevice = bin2bcd(LINUX_VERSION_MAJOR) << 8;
-	bcdDevice |= bin2bcd(LINUX_VERSION_PATCHLEVEL);
+	bcdDevice = bin2bcd((LINUX_VERSION_CODE >> 16 & 0xff)) << 8;
+	bcdDevice |= bin2bcd((LINUX_VERSION_CODE >> 8 & 0xff));
 	return bcdDevice;
 }
 
diff --git a/kernel/sys.c b/kernel/sys.c
index b09fe21e88ff5..8bb46e50f02d4 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -1242,7 +1242,7 @@ static int override_release(char __user *release, size_t len)
 				break;
 			rest++;
 		}
-		v = LINUX_VERSION_PATCHLEVEL + 60;
+		v = ((LINUX_VERSION_CODE >> 8) & 0xff) + 60;
 		copy = clamp_t(size_t, len, 1, sizeof(buf));
 		copy = scnprintf(buf, copy, "2.6.%u%s", v, rest);
 		ret = copy_to_user(release, buf, copy + 1);
diff --git a/tools/perf/tests/bpf-script-example.c b/tools/perf/tests/bpf-script-example.c
index a56bf381335e9..ab4b98b3165db 100644
--- a/tools/perf/tests/bpf-script-example.c
+++ b/tools/perf/tests/bpf-script-example.c
@@ -5,7 +5,7 @@
  */
 #ifndef LINUX_VERSION_CODE
 # error Need LINUX_VERSION_CODE
-# error Example: for 4.2 kernel, put 'clang-opt="-DLINUX_VERSION_CODE=0x4020000" into llvm section of ~/.perfconfig'
+# error Example: for 4.2 kernel, put 'clang-opt="-DLINUX_VERSION_CODE=0x40200" into llvm section of ~/.perfconfig'
 #endif
 #define BPF_ANY 0
 #define BPF_MAP_TYPE_ARRAY 2
diff --git a/tools/perf/tests/bpf-script-test-kbuild.c b/tools/perf/tests/bpf-script-test-kbuild.c
index 21663295d5b5a..219673aa278fb 100644
--- a/tools/perf/tests/bpf-script-test-kbuild.c
+++ b/tools/perf/tests/bpf-script-test-kbuild.c
@@ -5,7 +5,7 @@
  */
 #ifndef LINUX_VERSION_CODE
 # error Need LINUX_VERSION_CODE
-# error Example: for 4.2 kernel, put 'clang-opt="-DLINUX_VERSION_CODE=0x4020000" into llvm section of ~/.perfconfig'
+# error Example: for 4.2 kernel, put 'clang-opt="-DLINUX_VERSION_CODE=0x40200" into llvm section of ~/.perfconfig'
 #endif
 #define SEC(NAME) __attribute__((section(NAME), used))
 
diff --git a/tools/perf/tests/bpf-script-test-prologue.c b/tools/perf/tests/bpf-script-test-prologue.c
index 8db19e70813cc..bd83d364cf30d 100644
--- a/tools/perf/tests/bpf-script-test-prologue.c
+++ b/tools/perf/tests/bpf-script-test-prologue.c
@@ -5,7 +5,7 @@
  */
 #ifndef LINUX_VERSION_CODE
 # error Need LINUX_VERSION_CODE
-# error Example: for 4.2 kernel, put 'clang-opt="-DLINUX_VERSION_CODE=0x4020000" into llvm section of ~/.perfconfig'
+# error Example: for 4.2 kernel, put 'clang-opt="-DLINUX_VERSION_CODE=0x40200" into llvm section of ~/.perfconfig'
 #endif
 #define SEC(NAME) __attribute__((section(NAME), used))
 
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/3] kbuild: clamp SUBLEVEL to 255
  2021-02-06  3:50 [PATCH 1/3] Revert "kbuild: give the SUBLEVEL more room in KERNEL_VERSION" Sasha Levin
@ 2021-02-06  3:50 ` Sasha Levin
  2021-02-06 12:51   ` Greg KH
                     ` (2 more replies)
  2021-02-06  3:50 ` [PATCH 3/3] kbuild: introduce KERNEL_VERSION2 and LINUX_VERSION_CODE2 Sasha Levin
  2021-02-06 12:06 ` [PATCH 1/3] Revert "kbuild: give the SUBLEVEL more room in KERNEL_VERSION" Masahiro Yamada
  2 siblings, 3 replies; 15+ messages in thread
From: Sasha Levin @ 2021-02-06  3:50 UTC (permalink / raw)
  To: masahiroy, michal.lkml
  Cc: linux-kbuild, linux-kernel, torvalds, gregkh, Sasha Levin

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
 
 $(version_h): FORCE
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 3/3] kbuild: introduce KERNEL_VERSION2 and LINUX_VERSION_CODE2
  2021-02-06  3:50 [PATCH 1/3] Revert "kbuild: give the SUBLEVEL more room in KERNEL_VERSION" Sasha Levin
  2021-02-06  3:50 ` [PATCH 2/3] kbuild: clamp SUBLEVEL to 255 Sasha Levin
@ 2021-02-06  3:50 ` Sasha Levin
  2021-02-06  4:01   ` Sasha Levin
                     ` (3 more replies)
  2021-02-06 12:06 ` [PATCH 1/3] Revert "kbuild: give the SUBLEVEL more room in KERNEL_VERSION" Masahiro Yamada
  2 siblings, 4 replies; 15+ messages in thread
From: Sasha Levin @ 2021-02-06  3:50 UTC (permalink / raw)
  To: masahiroy, michal.lkml
  Cc: linux-kbuild, linux-kernel, torvalds, gregkh, Sasha Levin, stable

SUBLEVEL only has 8 bits of space, which means that we'll overflow it
once it reaches 256.

Few of the stable branches will imminently overflow SUBLEVEL while
there's no risk of overflowing VERSION.

Thus, give SUBLEVEL 8 more bits which will be stolen from VERSION, this
should create a better balance between the different version numbers we
use.

We can't however use the original KERNEL_VERSION and LINUX_VERSION_CODE
as userspace has created ABI dependency on their structure, and we risk
breaking this userspace by modifying the layout of the version integers.

Cc: stable@kernel.org
Signed-off-by: Sasha Levin <sashal@kernel.org>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 Makefile                                       | 8 +++++++-
 drivers/net/ethernet/mellanox/mlx5/core/main.c | 4 ++--
 drivers/usb/core/hcd.c                         | 4 ++--
 drivers/usb/gadget/udc/aspeed-vhub/hub.c       | 4 ++--
 include/linux/usb/composite.h                  | 4 ++--
 kernel/sys.c                                   | 2 +-
 tools/perf/tests/bpf-script-example.c          | 2 +-
 tools/perf/tests/bpf-script-test-kbuild.c      | 2 +-
 tools/perf/tests/bpf-script-test-prologue.c    | 2 +-
 9 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/Makefile b/Makefile
index 157be50c691e5..2177c548e4c24 100644
--- a/Makefile
+++ b/Makefile
@@ -1266,7 +1266,13 @@ define filechk_version.h
 		expr $(VERSION) \* 65536 + 0$(PATCHLEVEL) \* 256 + $(SUBLEVEL)); \
 	fi;                                                              \
 	echo '#define KERNEL_VERSION(a,b,c) (((a) << 16) + ((b) << 8) +  \
-	((c) > 255 ? 255 : (c)))'
+	((c) > 255 ? 255 : (c)))';                                       \
+	echo \#define LINUX_VERSION_CODE2 $(shell                        \
+	expr $(VERSION) \* 16777216 + 0$(PATCHLEVEL) \* 65536 + 0$(SUBLEVEL)); \
+	echo \#define LINUX_VERSION_MAJOR $(VERSION);                    \
+	echo \#define LINUX_VERSION_PATCHLEVEL $(PATCHLEVEL);            \
+	echo \#define LINUX_VERSION_SUBLEVEL $(SUBLEVEL);                \
+	echo '#define KERNEL_VERSION2(a,b,c) (((a) << 24) + ((b) << 16) + (c))'
 endef
 
 $(version_h): FORCE
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
index e4c9627485aa5..989f15d9aa7d4 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
@@ -237,8 +237,8 @@ static void mlx5_set_driver_version(struct mlx5_core_dev *dev)
 	remaining_size = max_t(int, 0, driver_ver_sz - strlen(string));
 
 	snprintf(string + strlen(string), remaining_size, "%u.%u.%u",
-		 (u8)((LINUX_VERSION_CODE >> 16) & 0xff), (u8)((LINUX_VERSION_CODE >> 8) & 0xff),
-		 (u16)(LINUX_VERSION_CODE & 0xffff));
+		(u8)(LINUX_VERSION_MAJOR), (u8)(LINUX_VERSION_PATCHLEVEL),
+		(u16)(LINUX_VERSION_SUBLEVEL));
 
 	/*Send the command*/
 	MLX5_SET(set_driver_version_in, in, opcode,
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index ad5a0f405a75c..3f0381344221e 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -111,8 +111,8 @@ DECLARE_WAIT_QUEUE_HEAD(usb_kill_urb_queue);
  */
 
 /*-------------------------------------------------------------------------*/
-#define KERNEL_REL	bin2bcd(((LINUX_VERSION_CODE >> 16) & 0x0ff))
-#define KERNEL_VER	bin2bcd(((LINUX_VERSION_CODE >> 8) & 0x0ff))
+#define KERNEL_REL	bin2bcd(LINUX_VERSION_MAJOR)
+#define KERNEL_VER	bin2bcd(LINUX_VERSION_PATCHLEVEL)
 
 /* usb 3.1 root hub device descriptor */
 static const u8 usb31_rh_dev_descriptor[18] = {
diff --git a/drivers/usb/gadget/udc/aspeed-vhub/hub.c b/drivers/usb/gadget/udc/aspeed-vhub/hub.c
index bfd8e77788e29..5c7dea5e0ff16 100644
--- a/drivers/usb/gadget/udc/aspeed-vhub/hub.c
+++ b/drivers/usb/gadget/udc/aspeed-vhub/hub.c
@@ -46,8 +46,8 @@
  *    - Make vid/did overridable
  *    - make it look like usb1 if usb1 mode forced
  */
-#define KERNEL_REL	bin2bcd(((LINUX_VERSION_CODE >> 16) & 0x0ff))
-#define KERNEL_VER	bin2bcd(((LINUX_VERSION_CODE >> 8) & 0x0ff))
+#define KERNEL_REL	bin2bcd(LINUX_VERSION_MAJOR)
+#define KERNEL_VER	bin2bcd(LINUX_VERSION_PATCHLEVEL)
 
 enum {
 	AST_VHUB_STR_INDEX_MAX = 4,
diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
index 5646dad886e61..c71150f2c6390 100644
--- a/include/linux/usb/composite.h
+++ b/include/linux/usb/composite.h
@@ -575,8 +575,8 @@ static inline u16 get_default_bcdDevice(void)
 {
 	u16 bcdDevice;
 
-	bcdDevice = bin2bcd((LINUX_VERSION_CODE >> 16 & 0xff)) << 8;
-	bcdDevice |= bin2bcd((LINUX_VERSION_CODE >> 8 & 0xff));
+	bcdDevice = bin2bcd(LINUX_VERSION_MAJOR) << 8;
+	bcdDevice |= bin2bcd(LINUX_VERSION_PATCHLEVEL);
 	return bcdDevice;
 }
 
diff --git a/kernel/sys.c b/kernel/sys.c
index 8bb46e50f02d4..b09fe21e88ff5 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -1242,7 +1242,7 @@ static int override_release(char __user *release, size_t len)
 				break;
 			rest++;
 		}
-		v = ((LINUX_VERSION_CODE >> 8) & 0xff) + 60;
+		v = LINUX_VERSION_PATCHLEVEL + 60;
 		copy = clamp_t(size_t, len, 1, sizeof(buf));
 		copy = scnprintf(buf, copy, "2.6.%u%s", v, rest);
 		ret = copy_to_user(release, buf, copy + 1);
diff --git a/tools/perf/tests/bpf-script-example.c b/tools/perf/tests/bpf-script-example.c
index ab4b98b3165db..a56bf381335e9 100644
--- a/tools/perf/tests/bpf-script-example.c
+++ b/tools/perf/tests/bpf-script-example.c
@@ -5,7 +5,7 @@
  */
 #ifndef LINUX_VERSION_CODE
 # error Need LINUX_VERSION_CODE
-# error Example: for 4.2 kernel, put 'clang-opt="-DLINUX_VERSION_CODE=0x40200" into llvm section of ~/.perfconfig'
+# error Example: for 4.2 kernel, put 'clang-opt="-DLINUX_VERSION_CODE=0x4020000" into llvm section of ~/.perfconfig'
 #endif
 #define BPF_ANY 0
 #define BPF_MAP_TYPE_ARRAY 2
diff --git a/tools/perf/tests/bpf-script-test-kbuild.c b/tools/perf/tests/bpf-script-test-kbuild.c
index 219673aa278fb..21663295d5b5a 100644
--- a/tools/perf/tests/bpf-script-test-kbuild.c
+++ b/tools/perf/tests/bpf-script-test-kbuild.c
@@ -5,7 +5,7 @@
  */
 #ifndef LINUX_VERSION_CODE
 # error Need LINUX_VERSION_CODE
-# error Example: for 4.2 kernel, put 'clang-opt="-DLINUX_VERSION_CODE=0x40200" into llvm section of ~/.perfconfig'
+# error Example: for 4.2 kernel, put 'clang-opt="-DLINUX_VERSION_CODE=0x4020000" into llvm section of ~/.perfconfig'
 #endif
 #define SEC(NAME) __attribute__((section(NAME), used))
 
diff --git a/tools/perf/tests/bpf-script-test-prologue.c b/tools/perf/tests/bpf-script-test-prologue.c
index bd83d364cf30d..8db19e70813cc 100644
--- a/tools/perf/tests/bpf-script-test-prologue.c
+++ b/tools/perf/tests/bpf-script-test-prologue.c
@@ -5,7 +5,7 @@
  */
 #ifndef LINUX_VERSION_CODE
 # error Need LINUX_VERSION_CODE
-# error Example: for 4.2 kernel, put 'clang-opt="-DLINUX_VERSION_CODE=0x40200" into llvm section of ~/.perfconfig'
+# error Example: for 4.2 kernel, put 'clang-opt="-DLINUX_VERSION_CODE=0x4020000" into llvm section of ~/.perfconfig'
 #endif
 #define SEC(NAME) __attribute__((section(NAME), used))
 
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/3] kbuild: introduce KERNEL_VERSION2 and LINUX_VERSION_CODE2
  2021-02-06  3:50 ` [PATCH 3/3] kbuild: introduce KERNEL_VERSION2 and LINUX_VERSION_CODE2 Sasha Levin
@ 2021-02-06  4:01   ` Sasha Levin
  2021-02-06 12:45   ` Masahiro Yamada
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 15+ messages in thread
From: Sasha Levin @ 2021-02-06  4:01 UTC (permalink / raw)
  To: masahiroy, michal.lkml
  Cc: linux-kbuild, linux-kernel, torvalds, gregkh, stable

On Fri, Feb 05, 2021 at 10:50:33PM -0500, Sasha Levin wrote:
>SUBLEVEL only has 8 bits of space, which means that we'll overflow it
>once it reaches 256.
>
>Few of the stable branches will imminently overflow SUBLEVEL while
>there's no risk of overflowing VERSION.
>
>Thus, give SUBLEVEL 8 more bits which will be stolen from VERSION, this
>should create a better balance between the different version numbers we
>use.
>
>We can't however use the original KERNEL_VERSION and LINUX_VERSION_CODE
>as userspace has created ABI dependency on their structure, and we risk
>breaking this userspace by modifying the layout of the version integers.
>
>Cc: stable@kernel.org
>Signed-off-by: Sasha Levin <sashal@kernel.org>
>Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

I wanted to re-use an older commit but forgot to drop the two tags
above. The tags from Masahiro and Greg shouldn't be here, sorry about
that.

-- 
Thanks,
Sasha

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/3] Revert "kbuild: give the SUBLEVEL more room in KERNEL_VERSION"
  2021-02-06  3:50 [PATCH 1/3] Revert "kbuild: give the SUBLEVEL more room in KERNEL_VERSION" Sasha Levin
  2021-02-06  3:50 ` [PATCH 2/3] kbuild: clamp SUBLEVEL to 255 Sasha Levin
  2021-02-06  3:50 ` [PATCH 3/3] kbuild: introduce KERNEL_VERSION2 and LINUX_VERSION_CODE2 Sasha Levin
@ 2021-02-06 12:06 ` Masahiro Yamada
  2 siblings, 0 replies; 15+ messages in thread
From: Masahiro Yamada @ 2021-02-06 12:06 UTC (permalink / raw)
  To: Sasha Levin
  Cc: Michal Marek, Linux Kbuild mailing list,
	Linux Kernel Mailing List, Linus Torvalds, Greg Kroah-Hartman

On Sat, Feb 6, 2021 at 12:50 PM Sasha Levin <sashal@kernel.org> wrote:
>
> This reverts commit 537896fabed11f8d9788886d1aacdb977213c7b3.
>
> This turns out to be a bad idea: userspace has coded the structure of
> KERNEL_VERSION on it's own and assumes the 2-1-1 byte split, making it
> userspace ABI we can't break.

It is unfortunate...

I will drop this from my tree.

This did not hit Linus' tree yet.



> The reverted patch didn't make it past linux-next, so no userspace was
> hurt in the process.
>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
>  Makefile                                       | 7 ++-----
>  drivers/net/ethernet/mellanox/mlx5/core/main.c | 4 ++--
>  drivers/usb/core/hcd.c                         | 4 ++--
>  drivers/usb/gadget/udc/aspeed-vhub/hub.c       | 4 ++--
>  include/linux/usb/composite.h                  | 4 ++--
>  kernel/sys.c                                   | 2 +-
>  tools/perf/tests/bpf-script-example.c          | 2 +-
>  tools/perf/tests/bpf-script-test-kbuild.c      | 2 +-
>  tools/perf/tests/bpf-script-test-prologue.c    | 2 +-
>  9 files changed, 14 insertions(+), 17 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 28019532e55ac..49ac1b7fe8e99 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1259,11 +1259,8 @@ endef
>
>  define filechk_version.h
>         echo \#define LINUX_VERSION_CODE $(shell                         \
> -       expr $(VERSION) \* 16777216 + 0$(PATCHLEVEL) \* 65536 + 0$(SUBLEVEL)); \
> -       echo \#define LINUX_VERSION_MAJOR $(VERSION); \
> -       echo \#define LINUX_VERSION_PATCHLEVEL $(PATCHLEVEL); \
> -       echo \#define LINUX_VERSION_SUBLEVEL $(SUBLEVEL); \
> -       echo '#define KERNEL_VERSION(a,b,c) (((a) << 24) + ((b) << 16) + (c))'
> +       expr $(VERSION) \* 65536 + 0$(PATCHLEVEL) \* 256 + 0$(SUBLEVEL)); \
> +       echo '#define KERNEL_VERSION(a,b,c) (((a) << 16) + ((b) << 8) + (c))'
>  endef
>
>  $(version_h): FORCE
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
> index 989f15d9aa7d4..e4c9627485aa5 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
> @@ -237,8 +237,8 @@ static void mlx5_set_driver_version(struct mlx5_core_dev *dev)
>         remaining_size = max_t(int, 0, driver_ver_sz - strlen(string));
>
>         snprintf(string + strlen(string), remaining_size, "%u.%u.%u",
> -               (u8)(LINUX_VERSION_MAJOR), (u8)(LINUX_VERSION_PATCHLEVEL),
> -               (u16)(LINUX_VERSION_SUBLEVEL));
> +                (u8)((LINUX_VERSION_CODE >> 16) & 0xff), (u8)((LINUX_VERSION_CODE >> 8) & 0xff),
> +                (u16)(LINUX_VERSION_CODE & 0xffff));
>
>         /*Send the command*/
>         MLX5_SET(set_driver_version_in, in, opcode,
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index 3f0381344221e..ad5a0f405a75c 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -111,8 +111,8 @@ DECLARE_WAIT_QUEUE_HEAD(usb_kill_urb_queue);
>   */
>
>  /*-------------------------------------------------------------------------*/
> -#define KERNEL_REL     bin2bcd(LINUX_VERSION_MAJOR)
> -#define KERNEL_VER     bin2bcd(LINUX_VERSION_PATCHLEVEL)
> +#define KERNEL_REL     bin2bcd(((LINUX_VERSION_CODE >> 16) & 0x0ff))
> +#define KERNEL_VER     bin2bcd(((LINUX_VERSION_CODE >> 8) & 0x0ff))
>
>  /* usb 3.1 root hub device descriptor */
>  static const u8 usb31_rh_dev_descriptor[18] = {
> diff --git a/drivers/usb/gadget/udc/aspeed-vhub/hub.c b/drivers/usb/gadget/udc/aspeed-vhub/hub.c
> index 5c7dea5e0ff16..bfd8e77788e29 100644
> --- a/drivers/usb/gadget/udc/aspeed-vhub/hub.c
> +++ b/drivers/usb/gadget/udc/aspeed-vhub/hub.c
> @@ -46,8 +46,8 @@
>   *    - Make vid/did overridable
>   *    - make it look like usb1 if usb1 mode forced
>   */
> -#define KERNEL_REL     bin2bcd(LINUX_VERSION_MAJOR)
> -#define KERNEL_VER     bin2bcd(LINUX_VERSION_PATCHLEVEL)
> +#define KERNEL_REL     bin2bcd(((LINUX_VERSION_CODE >> 16) & 0x0ff))
> +#define KERNEL_VER     bin2bcd(((LINUX_VERSION_CODE >> 8) & 0x0ff))
>
>  enum {
>         AST_VHUB_STR_INDEX_MAX = 4,
> diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
> index c71150f2c6390..5646dad886e61 100644
> --- a/include/linux/usb/composite.h
> +++ b/include/linux/usb/composite.h
> @@ -575,8 +575,8 @@ static inline u16 get_default_bcdDevice(void)
>  {
>         u16 bcdDevice;
>
> -       bcdDevice = bin2bcd(LINUX_VERSION_MAJOR) << 8;
> -       bcdDevice |= bin2bcd(LINUX_VERSION_PATCHLEVEL);
> +       bcdDevice = bin2bcd((LINUX_VERSION_CODE >> 16 & 0xff)) << 8;
> +       bcdDevice |= bin2bcd((LINUX_VERSION_CODE >> 8 & 0xff));
>         return bcdDevice;
>  }
>
> diff --git a/kernel/sys.c b/kernel/sys.c
> index b09fe21e88ff5..8bb46e50f02d4 100644
> --- a/kernel/sys.c
> +++ b/kernel/sys.c
> @@ -1242,7 +1242,7 @@ static int override_release(char __user *release, size_t len)
>                                 break;
>                         rest++;
>                 }
> -               v = LINUX_VERSION_PATCHLEVEL + 60;
> +               v = ((LINUX_VERSION_CODE >> 8) & 0xff) + 60;
>                 copy = clamp_t(size_t, len, 1, sizeof(buf));
>                 copy = scnprintf(buf, copy, "2.6.%u%s", v, rest);
>                 ret = copy_to_user(release, buf, copy + 1);
> diff --git a/tools/perf/tests/bpf-script-example.c b/tools/perf/tests/bpf-script-example.c
> index a56bf381335e9..ab4b98b3165db 100644
> --- a/tools/perf/tests/bpf-script-example.c
> +++ b/tools/perf/tests/bpf-script-example.c
> @@ -5,7 +5,7 @@
>   */
>  #ifndef LINUX_VERSION_CODE
>  # error Need LINUX_VERSION_CODE
> -# error Example: for 4.2 kernel, put 'clang-opt="-DLINUX_VERSION_CODE=0x4020000" into llvm section of ~/.perfconfig'
> +# error Example: for 4.2 kernel, put 'clang-opt="-DLINUX_VERSION_CODE=0x40200" into llvm section of ~/.perfconfig'
>  #endif
>  #define BPF_ANY 0
>  #define BPF_MAP_TYPE_ARRAY 2
> diff --git a/tools/perf/tests/bpf-script-test-kbuild.c b/tools/perf/tests/bpf-script-test-kbuild.c
> index 21663295d5b5a..219673aa278fb 100644
> --- a/tools/perf/tests/bpf-script-test-kbuild.c
> +++ b/tools/perf/tests/bpf-script-test-kbuild.c
> @@ -5,7 +5,7 @@
>   */
>  #ifndef LINUX_VERSION_CODE
>  # error Need LINUX_VERSION_CODE
> -# error Example: for 4.2 kernel, put 'clang-opt="-DLINUX_VERSION_CODE=0x4020000" into llvm section of ~/.perfconfig'
> +# error Example: for 4.2 kernel, put 'clang-opt="-DLINUX_VERSION_CODE=0x40200" into llvm section of ~/.perfconfig'
>  #endif
>  #define SEC(NAME) __attribute__((section(NAME), used))
>
> diff --git a/tools/perf/tests/bpf-script-test-prologue.c b/tools/perf/tests/bpf-script-test-prologue.c
> index 8db19e70813cc..bd83d364cf30d 100644
> --- a/tools/perf/tests/bpf-script-test-prologue.c
> +++ b/tools/perf/tests/bpf-script-test-prologue.c
> @@ -5,7 +5,7 @@
>   */
>  #ifndef LINUX_VERSION_CODE
>  # error Need LINUX_VERSION_CODE
> -# error Example: for 4.2 kernel, put 'clang-opt="-DLINUX_VERSION_CODE=0x4020000" into llvm section of ~/.perfconfig'
> +# error Example: for 4.2 kernel, put 'clang-opt="-DLINUX_VERSION_CODE=0x40200" into llvm section of ~/.perfconfig'
>  #endif
>  #define SEC(NAME) __attribute__((section(NAME), used))
>
> --
> 2.27.0
>


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/3] kbuild: introduce KERNEL_VERSION2 and LINUX_VERSION_CODE2
  2021-02-06  3:50 ` [PATCH 3/3] kbuild: introduce KERNEL_VERSION2 and LINUX_VERSION_CODE2 Sasha Levin
  2021-02-06  4:01   ` Sasha Levin
@ 2021-02-06 12:45   ` Masahiro Yamada
  2021-02-06 14:31     ` Sasha Levin
  2021-02-06 12:53   ` Greg KH
  2021-02-06 12:54   ` Greg KH
  3 siblings, 1 reply; 15+ messages in thread
From: Masahiro Yamada @ 2021-02-06 12:45 UTC (permalink / raw)
  To: Sasha Levin
  Cc: Michal Marek, Linux Kbuild mailing list,
	Linux Kernel Mailing List, Linus Torvalds, Greg Kroah-Hartman,
	stable

On Sat, Feb 6, 2021 at 12:50 PM Sasha Levin <sashal@kernel.org> wrote:
>
> SUBLEVEL only has 8 bits of space, which means that we'll overflow it
> once it reaches 256.
>
> Few of the stable branches will imminently overflow SUBLEVEL while
> there's no risk of overflowing VERSION.
>
> Thus, give SUBLEVEL 8 more bits which will be stolen from VERSION, this
> should create a better balance between the different version numbers we
> use.
>
> We can't however use the original KERNEL_VERSION and LINUX_VERSION_CODE
> as userspace has created ABI dependency on their structure, and we risk
> breaking this userspace by modifying the layout of the version integers.
>
> Cc: stable@kernel.org
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
>  Makefile                                       | 8 +++++++-
>  drivers/net/ethernet/mellanox/mlx5/core/main.c | 4 ++--
>  drivers/usb/core/hcd.c                         | 4 ++--
>  drivers/usb/gadget/udc/aspeed-vhub/hub.c       | 4 ++--
>  include/linux/usb/composite.h                  | 4 ++--
>  kernel/sys.c                                   | 2 +-
>  tools/perf/tests/bpf-script-example.c          | 2 +-
>  tools/perf/tests/bpf-script-test-kbuild.c      | 2 +-
>  tools/perf/tests/bpf-script-test-prologue.c    | 2 +-
>  9 files changed, 19 insertions(+), 13 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 157be50c691e5..2177c548e4c24 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1266,7 +1266,13 @@ define filechk_version.h
>                 expr $(VERSION) \* 65536 + 0$(PATCHLEVEL) \* 256 + $(SUBLEVEL)); \
>         fi;                                                              \
>         echo '#define KERNEL_VERSION(a,b,c) (((a) << 16) + ((b) << 8) +  \
> -       ((c) > 255 ? 255 : (c)))'
> +       ((c) > 255 ? 255 : (c)))';                                       \
> +       echo \#define LINUX_VERSION_CODE2 $(shell                        \
> +       expr $(VERSION) \* 16777216 + 0$(PATCHLEVEL) \* 65536 + 0$(SUBLEVEL)); \


Is this needed?
No one in this patch uses LINUX_VERSION_CODE2.

The in-kernel code can use
LINUX_VERSION_MAJOR/PATCHLEVEL/SUBLEVEL directly.


Userspace does not need LINUX_VERSION_CODE2 either.

Your comment in 2/3 states that
userspace only decodes MAJOR and PATCHLEVEL fields,
so LINUX_VERSION_CODE will still do well.




> index ab4b98b3165db..a56bf381335e9 100644
> --- a/tools/perf/tests/bpf-script-example.c
> +++ b/tools/perf/tests/bpf-script-example.c
> @@ -5,7 +5,7 @@
>   */
>  #ifndef LINUX_VERSION_CODE
>  # error Need LINUX_VERSION_CODE
> -# error Example: for 4.2 kernel, put 'clang-opt="-DLINUX_VERSION_CODE=0x40200" into llvm section of ~/.perfconfig'
> +# error Example: for 4.2 kernel, put 'clang-opt="-DLINUX_VERSION_CODE=0x4020000" into llvm section of ~/.perfconfig'


Is this change needed?




>  #endif
>  #define BPF_ANY 0
>  #define BPF_MAP_TYPE_ARRAY 2
> diff --git a/tools/perf/tests/bpf-script-test-kbuild.c b/tools/perf/tests/bpf-script-test-kbuild.c
> index 219673aa278fb..21663295d5b5a 100644
> --- a/tools/perf/tests/bpf-script-test-kbuild.c
> +++ b/tools/perf/tests/bpf-script-test-kbuild.c
> @@ -5,7 +5,7 @@
>   */
>  #ifndef LINUX_VERSION_CODE
>  # error Need LINUX_VERSION_CODE
> -# error Example: for 4.2 kernel, put 'clang-opt="-DLINUX_VERSION_CODE=0x40200" into llvm section of ~/.perfconfig'
> +# error Example: for 4.2 kernel, put 'clang-opt="-DLINUX_VERSION_CODE=0x4020000" into llvm section of ~/.perfconfig'
>  #endif
>  #define SEC(NAME) __attribute__((section(NAME), used))
>
> diff --git a/tools/perf/tests/bpf-script-test-prologue.c b/tools/perf/tests/bpf-script-test-prologue.c
> index bd83d364cf30d..8db19e70813cc 100644
> --- a/tools/perf/tests/bpf-script-test-prologue.c
> +++ b/tools/perf/tests/bpf-script-test-prologue.c
> @@ -5,7 +5,7 @@
>   */
>  #ifndef LINUX_VERSION_CODE
>  # error Need LINUX_VERSION_CODE
> -# error Example: for 4.2 kernel, put 'clang-opt="-DLINUX_VERSION_CODE=0x40200" into llvm section of ~/.perfconfig'
> +# error Example: for 4.2 kernel, put 'clang-opt="-DLINUX_VERSION_CODE=0x4020000" into llvm section of ~/.perfconfig'
>  #endif
>  #define SEC(NAME) __attribute__((section(NAME), used))
>
> --
> 2.27.0
>


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/3] kbuild: clamp SUBLEVEL to 255
  2021-02-06  3:50 ` [PATCH 2/3] kbuild: clamp SUBLEVEL to 255 Sasha Levin
@ 2021-02-06 12:51   ` Greg KH
  2021-02-07  2:52   ` Masahiro Yamada
  2021-02-08 13:48   ` David Laight
  2 siblings, 0 replies; 15+ messages in thread
From: Greg KH @ 2021-02-06 12:51 UTC (permalink / raw)
  To: Sasha Levin; +Cc: masahiroy, michal.lkml, linux-kbuild, linux-kernel, torvalds

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>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/3] kbuild: introduce KERNEL_VERSION2 and LINUX_VERSION_CODE2
  2021-02-06  3:50 ` [PATCH 3/3] kbuild: introduce KERNEL_VERSION2 and LINUX_VERSION_CODE2 Sasha Levin
  2021-02-06  4:01   ` Sasha Levin
  2021-02-06 12:45   ` Masahiro Yamada
@ 2021-02-06 12:53   ` Greg KH
  2021-02-06 12:54   ` Greg KH
  3 siblings, 0 replies; 15+ messages in thread
From: Greg KH @ 2021-02-06 12:53 UTC (permalink / raw)
  To: Sasha Levin
  Cc: masahiroy, michal.lkml, linux-kbuild, linux-kernel, torvalds, stable

On Fri, Feb 05, 2021 at 10:50:33PM -0500, Sasha Levin wrote:
> SUBLEVEL only has 8 bits of space, which means that we'll overflow it
> once it reaches 256.
> 
> Few of the stable branches will imminently overflow SUBLEVEL while
> there's no risk of overflowing VERSION.
> 
> Thus, give SUBLEVEL 8 more bits which will be stolen from VERSION, this
> should create a better balance between the different version numbers we
> use.
> 
> We can't however use the original KERNEL_VERSION and LINUX_VERSION_CODE
> as userspace has created ABI dependency on their structure, and we risk
> breaking this userspace by modifying the layout of the version integers.
> 
> Cc: stable@kernel.org
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> Signed-off-by: Sasha Levin <sashal@kernel.org>

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/3] kbuild: introduce KERNEL_VERSION2 and LINUX_VERSION_CODE2
  2021-02-06  3:50 ` [PATCH 3/3] kbuild: introduce KERNEL_VERSION2 and LINUX_VERSION_CODE2 Sasha Levin
                     ` (2 preceding siblings ...)
  2021-02-06 12:53   ` Greg KH
@ 2021-02-06 12:54   ` Greg KH
  3 siblings, 0 replies; 15+ messages in thread
From: Greg KH @ 2021-02-06 12:54 UTC (permalink / raw)
  To: Sasha Levin
  Cc: masahiroy, michal.lkml, linux-kbuild, linux-kernel, torvalds, stable

On Fri, Feb 05, 2021 at 10:50:33PM -0500, Sasha Levin wrote:
> SUBLEVEL only has 8 bits of space, which means that we'll overflow it
> once it reaches 256.
> 
> Few of the stable branches will imminently overflow SUBLEVEL while
> there's no risk of overflowing VERSION.
> 
> Thus, give SUBLEVEL 8 more bits which will be stolen from VERSION, this
> should create a better balance between the different version numbers we
> use.
> 
> We can't however use the original KERNEL_VERSION and LINUX_VERSION_CODE
> as userspace has created ABI dependency on their structure, and we risk
> breaking this userspace by modifying the layout of the version integers.
> 
> Cc: stable@kernel.org
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
>  Makefile                                       | 8 +++++++-
>  drivers/net/ethernet/mellanox/mlx5/core/main.c | 4 ++--
>  drivers/usb/core/hcd.c                         | 4 ++--
>  drivers/usb/gadget/udc/aspeed-vhub/hub.c       | 4 ++--
>  include/linux/usb/composite.h                  | 4 ++--
>  kernel/sys.c                                   | 2 +-
>  tools/perf/tests/bpf-script-example.c          | 2 +-
>  tools/perf/tests/bpf-script-test-kbuild.c      | 2 +-
>  tools/perf/tests/bpf-script-test-prologue.c    | 2 +-
>  9 files changed, 19 insertions(+), 13 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 157be50c691e5..2177c548e4c24 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1266,7 +1266,13 @@ define filechk_version.h
>  		expr $(VERSION) \* 65536 + 0$(PATCHLEVEL) \* 256 + $(SUBLEVEL)); \
>  	fi;                                                              \
>  	echo '#define KERNEL_VERSION(a,b,c) (((a) << 16) + ((b) << 8) +  \
> -	((c) > 255 ? 255 : (c)))'
> +	((c) > 255 ? 255 : (c)))';                                       \
> +	echo \#define LINUX_VERSION_CODE2 $(shell                        \

Wait, no, who will use LINUX_VERSION_CODE2()?  Why export something like
this to userspace that no one is asking for?

> +	expr $(VERSION) \* 16777216 + 0$(PATCHLEVEL) \* 65536 + 0$(SUBLEVEL)); \
> +	echo \#define LINUX_VERSION_MAJOR $(VERSION);                    \
> +	echo \#define LINUX_VERSION_PATCHLEVEL $(PATCHLEVEL);            \
> +	echo \#define LINUX_VERSION_SUBLEVEL $(SUBLEVEL);                \

These are good to have, and clean up kernel code.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/3] kbuild: introduce KERNEL_VERSION2 and LINUX_VERSION_CODE2
  2021-02-06 12:45   ` Masahiro Yamada
@ 2021-02-06 14:31     ` Sasha Levin
  0 siblings, 0 replies; 15+ messages in thread
From: Sasha Levin @ 2021-02-06 14:31 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Michal Marek, Linux Kbuild mailing list,
	Linux Kernel Mailing List, Linus Torvalds, Greg Kroah-Hartman,
	stable

On Sat, Feb 06, 2021 at 09:45:44PM +0900, Masahiro Yamada wrote:
>On Sat, Feb 6, 2021 at 12:50 PM Sasha Levin <sashal@kernel.org> wrote:
>> -       ((c) > 255 ? 255 : (c)))'
>> +       ((c) > 255 ? 255 : (c)))';                                       \
>> +       echo \#define LINUX_VERSION_CODE2 $(shell                        \
>> +       expr $(VERSION) \* 16777216 + 0$(PATCHLEVEL) \* 65536 + 0$(SUBLEVEL)); \
>
>
>Is this needed?
>No one in this patch uses LINUX_VERSION_CODE2.
>
>The in-kernel code can use
>LINUX_VERSION_MAJOR/PATCHLEVEL/SUBLEVEL directly.
>
>
>Userspace does not need LINUX_VERSION_CODE2 either.
>
>Your comment in 2/3 states that
>userspace only decodes MAJOR and PATCHLEVEL fields,
>so LINUX_VERSION_CODE will still do well.

Yes, that's fair. I wanted to have that to make it easy for out of tree
code to handle in-kernel ABI changes but as we don't care about such
code there's no strict need for LINUX_VERSION_CODE2. I'll drop it.

>> index ab4b98b3165db..a56bf381335e9 100644
>> --- a/tools/perf/tests/bpf-script-example.c
>> +++ b/tools/perf/tests/bpf-script-example.c
>> @@ -5,7 +5,7 @@
>>   */
>>  #ifndef LINUX_VERSION_CODE
>>  # error Need LINUX_VERSION_CODE
>> -# error Example: for 4.2 kernel, put 'clang-opt="-DLINUX_VERSION_CODE=0x40200" into llvm section of ~/.perfconfig'
>> +# error Example: for 4.2 kernel, put 'clang-opt="-DLINUX_VERSION_CODE=0x4020000" into llvm section of ~/.perfconfig'
>
>
>Is this change needed?

Good point, no. I'll resend.

-- 
Thanks,
Sasha

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/3] kbuild: clamp SUBLEVEL to 255
  2021-02-06  3:50 ` [PATCH 2/3] kbuild: clamp SUBLEVEL to 255 Sasha Levin
  2021-02-06 12:51   ` Greg KH
@ 2021-02-07  2:52   ` Masahiro Yamada
  2021-02-08 13:48   ` David Laight
  2 siblings, 0 replies; 15+ messages in thread
From: Masahiro Yamada @ 2021-02-07  2:52 UTC (permalink / raw)
  To: Sasha Levin
  Cc: Michal Marek, Linux Kbuild mailing list,
	Linux Kernel Mailing List, Linus Torvalds, Greg Kroah-Hartman

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
>


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [PATCH 2/3] kbuild: clamp SUBLEVEL to 255
  2021-02-06  3:50 ` [PATCH 2/3] kbuild: clamp SUBLEVEL to 255 Sasha Levin
  2021-02-06 12:51   ` Greg KH
  2021-02-07  2:52   ` Masahiro Yamada
@ 2021-02-08 13:48   ` David Laight
  2021-02-08 14:09     ` gregkh
  2021-02-09  1:58     ` Masahiro Yamada
  2 siblings, 2 replies; 15+ messages in thread
From: David Laight @ 2021-02-08 13:48 UTC (permalink / raw)
  To: 'Sasha Levin', masahiroy, michal.lkml
  Cc: linux-kbuild, linux-kernel, torvalds, gregkh

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)


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/3] kbuild: clamp SUBLEVEL to 255
  2021-02-08 13:48   ` David Laight
@ 2021-02-08 14:09     ` gregkh
  2021-02-08 15:13       ` David Laight
  2021-02-09  1:58     ` Masahiro Yamada
  1 sibling, 1 reply; 15+ messages in thread
From: gregkh @ 2021-02-08 14:09 UTC (permalink / raw)
  To: David Laight
  Cc: 'Sasha Levin',
	masahiroy, michal.lkml, linux-kbuild, linux-kernel, torvalds

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [PATCH 2/3] kbuild: clamp SUBLEVEL to 255
  2021-02-08 14:09     ` gregkh
@ 2021-02-08 15:13       ` David Laight
  0 siblings, 0 replies; 15+ messages in thread
From: David Laight @ 2021-02-08 15:13 UTC (permalink / raw)
  To: 'gregkh@linuxfoundation.org'
  Cc: 'Sasha Levin',
	masahiroy, michal.lkml, linux-kbuild, linux-kernel, torvalds

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)


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/3] kbuild: clamp SUBLEVEL to 255
  2021-02-08 13:48   ` David Laight
  2021-02-08 14:09     ` gregkh
@ 2021-02-09  1:58     ` Masahiro Yamada
  1 sibling, 0 replies; 15+ messages in thread
From: Masahiro Yamada @ 2021-02-09  1:58 UTC (permalink / raw)
  To: David Laight
  Cc: Sasha Levin, michal.lkml, linux-kbuild, linux-kernel, torvalds, gregkh

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2021-02-09  2:00 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-06  3:50 [PATCH 1/3] Revert "kbuild: give the SUBLEVEL more room in KERNEL_VERSION" Sasha Levin
2021-02-06  3:50 ` [PATCH 2/3] kbuild: clamp SUBLEVEL to 255 Sasha Levin
2021-02-06 12:51   ` Greg KH
2021-02-07  2:52   ` Masahiro Yamada
2021-02-08 13:48   ` David Laight
2021-02-08 14:09     ` gregkh
2021-02-08 15:13       ` David Laight
2021-02-09  1:58     ` Masahiro Yamada
2021-02-06  3:50 ` [PATCH 3/3] kbuild: introduce KERNEL_VERSION2 and LINUX_VERSION_CODE2 Sasha Levin
2021-02-06  4:01   ` Sasha Levin
2021-02-06 12:45   ` Masahiro Yamada
2021-02-06 14:31     ` Sasha Levin
2021-02-06 12:53   ` Greg KH
2021-02-06 12:54   ` Greg KH
2021-02-06 12:06 ` [PATCH 1/3] Revert "kbuild: give the SUBLEVEL more room in KERNEL_VERSION" Masahiro Yamada

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).