[PATCH net-next v1 0/4] Remove vermagic header from global include folder

[PATCH net-next v1 0/4] Remove vermagic header from global include folder

[Leon Romanovsky]

From: Leon Romanovsky <leonro@mellanox.com>

Changelog:
v1:
 * Added tags
 * Updated patch #4 with test results
 * Changed scripts/mod/modpost.c to create inclusion of vermagic.h
   from kernel folder and not from general include/linux. This is
   needed to generate *.mod.c files, while building modules.
v0: https://lore.kernel.org/lkml/20200414155732.1236944-1-leon@kernel.org
----------------------------------------------------------------------------

Hi,

This is followup to the failure reported by Borislav [1] and suggested
fix later on [2].

The series removes all includes of linux/vermagic.h, updates hns and
nfp to use same kernel versioning scheme (exactly like we did for
other drivers in previous cycle) and removes vermagic.h from global
include folder.

[1] https://lore.kernel.org/lkml/20200411155623.GA22175@zn.tnic
[2] https://lore.kernel.org/lkml/20200413080452.GA3772@zn.tnic

------------------------------------------------------------
1. Honestly, I have no idea if it can go to net-rc, clearly not all my
patches are fixes, so I'm sending them to the net-next.
2. Still didn't get response from kbuild, but it passed my own
compilation tests.
https://git.kernel.org/pub/scm/linux/kernel/git/leon/linux-rdma.git/log/?h=vermagic

Thanks

Leon Romanovsky (4):
  drivers: Remove inclusion of vermagic header
  net/hns: Remove custom driver version in favour of global one
  net/nfp: Update driver to use global kernel version
  kernel/module: Hide vermagic header file from general use

 drivers/net/bonding/bonding_priv.h                   | 2 +-
 drivers/net/ethernet/3com/3c509.c                    | 1 -
 drivers/net/ethernet/3com/3c515.c                    | 1 -
 drivers/net/ethernet/adaptec/starfire.c              | 1 -
 drivers/net/ethernet/hisilicon/hns3/hns3_enet.c      | 3 ---
 drivers/net/ethernet/hisilicon/hns3/hns3_enet.h      | 4 ----
 drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c   | 4 ----
 drivers/net/ethernet/netronome/nfp/nfp_main.c        | 3 ---
 drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c | 2 --
 drivers/net/ethernet/pensando/ionic/ionic_main.c     | 2 +-
 drivers/power/supply/test_power.c                    | 2 +-
 kernel/module.c                                      | 2 +-
 {include/linux => kernel}/vermagic.h                 | 0
 net/ethtool/ioctl.c                                  | 3 +--
 scripts/mod/modpost.c                                | 2 +-
 15 files changed, 6 insertions(+), 26 deletions(-)
 rename {include/linux => kernel}/vermagic.h (100%)

--
2.25.2

[PATCH net-next v1 1/4] drivers: Remove inclusion of vermagic header

[Leon Romanovsky]

From: Leon Romanovsky <leonro@mellanox.com>

Get rid of linux/vermagic.h includes, so that MODULE_ARCH_VERMAGIC from
the arch header arch/x86/include/asm/module.h won't be redefined.

  In file included from ./include/linux/module.h:30,
                   from drivers/net/ethernet/3com/3c515.c:56:
  ./arch/x86/include/asm/module.h:73: warning: "MODULE_ARCH_VERMAGIC"
redefined
     73 | # define MODULE_ARCH_VERMAGIC MODULE_PROC_FAMILY
        |
  In file included from drivers/net/ethernet/3com/3c515.c:25:
  ./include/linux/vermagic.h:28: note: this is the location of the
previous definition
     28 | #define MODULE_ARCH_VERMAGIC ""
        |

Fixes: 6bba2e89a88c ("net/3com: Delete driver and module versions from 3com drivers")
Co-developed-by: Borislav Petkov <bp@suse.de>
Signed-off-by: Borislav Petkov <bp@suse.de>
Acked-by: Shannon Nelson <snelson@pensando.io> # ionic
Acked-by: Sebastian Reichel <sre@kernel.org> # power
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/net/bonding/bonding_priv.h               | 2 +-
 drivers/net/ethernet/3com/3c509.c                | 1 -
 drivers/net/ethernet/3com/3c515.c                | 1 -
 drivers/net/ethernet/adaptec/starfire.c          | 1 -
 drivers/net/ethernet/pensando/ionic/ionic_main.c | 2 +-
 drivers/power/supply/test_power.c                | 2 +-
 net/ethtool/ioctl.c                              | 3 +--
 7 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/net/bonding/bonding_priv.h b/drivers/net/bonding/bonding_priv.h
index 45b77bc8c7b3..48cdf3a49a7d 100644
--- a/drivers/net/bonding/bonding_priv.h
+++ b/drivers/net/bonding/bonding_priv.h
@@ -14,7 +14,7 @@

 #ifndef _BONDING_PRIV_H
 #define _BONDING_PRIV_H
-#include <linux/vermagic.h>
+#include <generated/utsrelease.h>

 #define DRV_NAME	"bonding"
 #define DRV_DESCRIPTION	"Ethernet Channel Bonding Driver"
diff --git a/drivers/net/ethernet/3com/3c509.c b/drivers/net/ethernet/3com/3c509.c
index b762176a1406..139d0120f511 100644
--- a/drivers/net/ethernet/3com/3c509.c
+++ b/drivers/net/ethernet/3com/3c509.c
@@ -85,7 +85,6 @@
 #include <linux/device.h>
 #include <linux/eisa.h>
 #include <linux/bitops.h>
-#include <linux/vermagic.h>

 #include <linux/uaccess.h>
 #include <asm/io.h>
diff --git a/drivers/net/ethernet/3com/3c515.c b/drivers/net/ethernet/3com/3c515.c
index 90312fcd6319..47b4215bb93b 100644
--- a/drivers/net/ethernet/3com/3c515.c
+++ b/drivers/net/ethernet/3com/3c515.c
@@ -22,7 +22,6 @@

 */

-#include <linux/vermagic.h>
 #define DRV_NAME		"3c515"

 #define CORKSCREW 1
diff --git a/drivers/net/ethernet/adaptec/starfire.c b/drivers/net/ethernet/adaptec/starfire.c
index 2db42211329f..a64191fc2af9 100644
--- a/drivers/net/ethernet/adaptec/starfire.c
+++ b/drivers/net/ethernet/adaptec/starfire.c
@@ -45,7 +45,6 @@
 #include <asm/processor.h>		/* Processor type for cache alignment. */
 #include <linux/uaccess.h>
 #include <asm/io.h>
-#include <linux/vermagic.h>

 /*
  * The current frame processor firmware fails to checksum a fragment
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_main.c b/drivers/net/ethernet/pensando/ionic/ionic_main.c
index 588c62e9add7..3ed150512091 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_main.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_main.c
@@ -6,7 +6,7 @@
 #include <linux/module.h>
 #include <linux/netdevice.h>
 #include <linux/utsname.h>
-#include <linux/vermagic.h>
+#include <generated/utsrelease.h>

 #include "ionic.h"
 #include "ionic_bus.h"
diff --git a/drivers/power/supply/test_power.c b/drivers/power/supply/test_power.c
index 65c23ef6408d..b3c05ff05783 100644
--- a/drivers/power/supply/test_power.c
+++ b/drivers/power/supply/test_power.c
@@ -16,7 +16,7 @@
 #include <linux/power_supply.h>
 #include <linux/errno.h>
 #include <linux/delay.h>
-#include <linux/vermagic.h>
+#include <generated/utsrelease.h>

 enum test_power_id {
 	TEST_AC,
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index 89d0b1827aaf..d3cb5a49a0ce 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -17,7 +17,6 @@
 #include <linux/phy.h>
 #include <linux/bitops.h>
 #include <linux/uaccess.h>
-#include <linux/vermagic.h>
 #include <linux/vmalloc.h>
 #include <linux/sfp.h>
 #include <linux/slab.h>
@@ -28,7 +27,7 @@
 #include <net/xdp_sock.h>
 #include <net/flow_offload.h>
 #include <linux/ethtool_netlink.h>
-
+#include <generated/utsrelease.h>
 #include "common.h"

 /*
--
2.25.2

[PATCH net-next v1 2/4] net/hns: Remove custom driver version in favour of global one

[Leon Romanovsky]

From: Leon Romanovsky <leonro@mellanox.com>

Use globally defined kernel version instead of custom driver variant.

Reported-by: Borislav Petkov <bp@suse.de>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/net/ethernet/hisilicon/hns3/hns3_enet.c    | 3 ---
 drivers/net/ethernet/hisilicon/hns3/hns3_enet.h    | 4 ----
 drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c | 4 ----
 3 files changed, 11 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index da98fd7c8eca..ac3a48a24d86 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -15,7 +15,6 @@
 #include <linux/aer.h>
 #include <linux/skbuff.h>
 #include <linux/sctp.h>
-#include <linux/vermagic.h>
 #include <net/gre.h>
 #include <net/ip6_checksum.h>
 #include <net/pkt_cls.h>
@@ -44,7 +43,6 @@ static void hns3_clear_all_ring(struct hnae3_handle *h, bool force);
 static void hns3_remove_hw_addr(struct net_device *netdev);
 
 static const char hns3_driver_name[] = "hns3";
-const char hns3_driver_version[] = VERMAGIC_STRING;
 static const char hns3_driver_string[] =
 			"Hisilicon Ethernet Network Driver for Hip08 Family";
 static const char hns3_copyright[] = "Copyright (c) 2017 Huawei Corporation.";
@@ -4765,4 +4763,3 @@ MODULE_DESCRIPTION("HNS3: Hisilicon Ethernet Driver");
 MODULE_AUTHOR("Huawei Tech. Co., Ltd.");
 MODULE_LICENSE("GPL");
 MODULE_ALIAS("pci:hns-nic");
-MODULE_VERSION(HNS3_MOD_VERSION);
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
index abefd7a179f7..4b3f0abf0715 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
@@ -8,10 +8,6 @@
 
 #include "hnae3.h"
 
-#define HNS3_MOD_VERSION "1.0"
-
-extern const char hns3_driver_version[];
-
 enum hns3_nic_state {
 	HNS3_NIC_STATE_TESTING,
 	HNS3_NIC_STATE_RESETTING,
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
index 28b81f24afa1..6a0734be4a1a 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
@@ -546,10 +546,6 @@ static void hns3_get_drvinfo(struct net_device *netdev,
 		return;
 	}
 
-	strncpy(drvinfo->version, hns3_driver_version,
-		sizeof(drvinfo->version));
-	drvinfo->version[sizeof(drvinfo->version) - 1] = '\0';
-
 	strncpy(drvinfo->driver, h->pdev->driver->name,
 		sizeof(drvinfo->driver));
 	drvinfo->driver[sizeof(drvinfo->driver) - 1] = '\0';
-- 
2.25.2

[PATCH net-next v1 3/4] net/nfp: Update driver to use global kernel version

[Leon Romanovsky]

From: Leon Romanovsky <leonro@mellanox.com>

Change nfp driver to use globally defined kernel version.

Reported-by: Borislav Petkov <bp@suse.de>
Acked-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/net/ethernet/netronome/nfp/nfp_main.c        | 3 ---
 drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c | 2 --
 2 files changed, 5 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_main.c b/drivers/net/ethernet/netronome/nfp/nfp_main.c
index 4d282fc56009..7ff2ccbd43b0 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_main.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_main.c
@@ -14,7 +14,6 @@
 #include <linux/mutex.h>
 #include <linux/pci.h>
 #include <linux/firmware.h>
-#include <linux/vermagic.h>
 #include <linux/vmalloc.h>
 #include <net/devlink.h>
 
@@ -31,7 +30,6 @@
 #include "nfp_net.h"
 
 static const char nfp_driver_name[] = "nfp";
-const char nfp_driver_version[] = VERMAGIC_STRING;
 
 static const struct pci_device_id nfp_pci_device_ids[] = {
 	{ PCI_VENDOR_ID_NETRONOME, PCI_DEVICE_ID_NETRONOME_NFP6000,
@@ -920,4 +918,3 @@ MODULE_FIRMWARE("netronome/nic_AMDA0099-0001_1x10_1x25.nffw");
 MODULE_AUTHOR("Netronome Systems <oss-drivers@netronome.com>");
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("The Netronome Flow Processor (NFP) driver.");
-MODULE_VERSION(UTS_RELEASE);
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
index 2779f1526d1e..a5aa3219d112 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
@@ -203,8 +203,6 @@ nfp_get_drvinfo(struct nfp_app *app, struct pci_dev *pdev,
 	char nsp_version[ETHTOOL_FWVERS_LEN] = {};
 
 	strlcpy(drvinfo->driver, pdev->driver->name, sizeof(drvinfo->driver));
-	strlcpy(drvinfo->version, nfp_driver_version, sizeof(drvinfo->version));
-
 	nfp_net_get_nspinfo(app, nsp_version);
 	snprintf(drvinfo->fw_version, sizeof(drvinfo->fw_version),
 		 "%s %s %s %s", vnic_version, nsp_version,
-- 
2.25.2

[PATCH net-next v1 4/4] kernel/module: Hide vermagic header file from general use

[Leon Romanovsky]

From: Leon Romanovsky <leonro@mellanox.com>

VERMAGIC* definitions are not supposed to be used by the drivers,
see this [1] bug report, so simply move this header file to be visible
to kernel/* and scripts files only.

In-tree module build:
➜  kernel git:(vermagic) ✗ make clean
➜  kernel git:(vermagic) ✗ make M=drivers/infiniband/hw/mlx5
➜  kernel git:(vermagic) ✗ modinfo drivers/infiniband/hw/mlx5/mlx5_ib.ko
filename:	/images/leonro/src/kernel/drivers/infiniband/hw/mlx5/mlx5_ib.ko
<...>
vermagic:       5.6.0+ SMP mod_unload modversions

Out-of-tree module build:
➜  mlx5 make -C /images/leonro/src/kernel clean M=/tmp/mlx5
➜  mlx5 make -C /images/leonro/src/kernel M=/tmp/mlx5
➜  mlx5 modinfo /tmp/mlx5/mlx5_ib.ko
filename:       /tmp/mlx5/mlx5_ib.ko
<...>
vermagic:       5.6.0+ SMP mod_unload modversions

[1] https://lore.kernel.org/lkml/20200411155623.GA22175@zn.tnic
Reported-by: Borislav Petkov <bp@suse.de>
Acked-by: Borislav Petkov <bp@suse.de>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 kernel/module.c                      | 2 +-
 {include/linux => kernel}/vermagic.h | 0
 scripts/mod/modpost.c                | 2 +-
 3 files changed, 2 insertions(+), 2 deletions(-)
 rename {include/linux => kernel}/vermagic.h (100%)

diff --git a/kernel/module.c b/kernel/module.c
index 3447f3b74870..fce06095d341 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -29,7 +29,6 @@
 #include <linux/moduleparam.h>
 #include <linux/errno.h>
 #include <linux/err.h>
-#include <linux/vermagic.h>
 #include <linux/notifier.h>
 #include <linux/sched.h>
 #include <linux/device.h>
@@ -55,6 +54,7 @@
 #include <linux/audit.h>
 #include <uapi/linux/module.h>
 #include "module-internal.h"
+#include "vermagic.h"

 #define CREATE_TRACE_POINTS
 #include <trace/events/module.h>
diff --git a/include/linux/vermagic.h b/kernel/vermagic.h
similarity index 100%
rename from include/linux/vermagic.h
rename to kernel/vermagic.h
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 5c3c50c5ec52..91f86261bcfe 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -2252,7 +2252,7 @@ static void add_header(struct buffer *b, struct module *mod)
 	 * inherit the definitions.
 	 */
 	buf_printf(b, "#include <linux/build-salt.h>\n");
-	buf_printf(b, "#include <linux/vermagic.h>\n");
+	buf_printf(b, "#include <../kernel/vermagic.h>\n");
 	buf_printf(b, "#include <linux/compiler.h>\n");
 	buf_printf(b, "\n");
 	buf_printf(b, "BUILD_SALT;\n");
--
2.25.2

Re: [PATCH net-next v1 4/4] kernel/module: Hide vermagic header file from general use

[Jessica Yu]

+++ Leon Romanovsky [15/04/20 16:36 +0300]:
>From: Leon Romanovsky <leonro@mellanox.com>
>
>VERMAGIC* definitions are not supposed to be used by the drivers,
>see this [1] bug report, so simply move this header file to be visible
>to kernel/* and scripts files only.
>
>In-tree module build:
>➜  kernel git:(vermagic) ✗ make clean
>➜  kernel git:(vermagic) ✗ make M=drivers/infiniband/hw/mlx5
>➜  kernel git:(vermagic) ✗ modinfo drivers/infiniband/hw/mlx5/mlx5_ib.ko
>filename:	/images/leonro/src/kernel/drivers/infiniband/hw/mlx5/mlx5_ib.ko
><...>
>vermagic:       5.6.0+ SMP mod_unload modversions
>
>Out-of-tree module build:
>➜  mlx5 make -C /images/leonro/src/kernel clean M=/tmp/mlx5
>➜  mlx5 make -C /images/leonro/src/kernel M=/tmp/mlx5
>➜  mlx5 modinfo /tmp/mlx5/mlx5_ib.ko
>filename:       /tmp/mlx5/mlx5_ib.ko
><...>
>vermagic:       5.6.0+ SMP mod_unload modversions
>
>[1] https://lore.kernel.org/lkml/20200411155623.GA22175@zn.tnic
>Reported-by: Borislav Petkov <bp@suse.de>
>Acked-by: Borislav Petkov <bp@suse.de>
>Signed-off-by: Leon Romanovsky <leonro@mellanox.com>

Acked-by: Jessica Yu <jeyu@kernel.org>

Re: [PATCH net-next v1 4/4] kernel/module: Hide vermagic header file from general use

[Masahiro Yamada]

On Wed, Apr 15, 2020 at 10:37 PM Leon Romanovsky <leon@kernel.org> wrote:
>
> From: Leon Romanovsky <leonro@mellanox.com>
>
> VERMAGIC* definitions are not supposed to be used by the drivers,
> see this [1] bug report, so simply move this header file to be visible
> to kernel/* and scripts files only.
>
> In-tree module build:
> ➜  kernel git:(vermagic) ✗ make clean
> ➜  kernel git:(vermagic) ✗ make M=drivers/infiniband/hw/mlx5
> ➜  kernel git:(vermagic) ✗ modinfo drivers/infiniband/hw/mlx5/mlx5_ib.ko
> filename:       /images/leonro/src/kernel/drivers/infiniband/hw/mlx5/mlx5_ib.ko
> <...>
> vermagic:       5.6.0+ SMP mod_unload modversions
>
> Out-of-tree module build:
> ➜  mlx5 make -C /images/leonro/src/kernel clean M=/tmp/mlx5
> ➜  mlx5 make -C /images/leonro/src/kernel M=/tmp/mlx5
> ➜  mlx5 modinfo /tmp/mlx5/mlx5_ib.ko
> filename:       /tmp/mlx5/mlx5_ib.ko
> <...>
> vermagic:       5.6.0+ SMP mod_unload modversions
>
> [1] https://lore.kernel.org/lkml/20200411155623.GA22175@zn.tnic
> Reported-by: Borislav Petkov <bp@suse.de>
> Acked-by: Borislav Petkov <bp@suse.de>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> ---
>  kernel/module.c                      | 2 +-
>  {include/linux => kernel}/vermagic.h | 0
>  scripts/mod/modpost.c                | 2 +-
>  3 files changed, 2 insertions(+), 2 deletions(-)
>  rename {include/linux => kernel}/vermagic.h (100%)
>
> diff --git a/kernel/module.c b/kernel/module.c
> index 3447f3b74870..fce06095d341 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -29,7 +29,6 @@
>  #include <linux/moduleparam.h>
>  #include <linux/errno.h>
>  #include <linux/err.h>
> -#include <linux/vermagic.h>
>  #include <linux/notifier.h>
>  #include <linux/sched.h>
>  #include <linux/device.h>
> @@ -55,6 +54,7 @@
>  #include <linux/audit.h>
>  #include <uapi/linux/module.h>
>  #include "module-internal.h"
> +#include "vermagic.h"
>
>  #define CREATE_TRACE_POINTS
>  #include <trace/events/module.h>
> diff --git a/include/linux/vermagic.h b/kernel/vermagic.h
> similarity index 100%
> rename from include/linux/vermagic.h
> rename to kernel/vermagic.h
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 5c3c50c5ec52..91f86261bcfe 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -2252,7 +2252,7 @@ static void add_header(struct buffer *b, struct module *mod)
>          * inherit the definitions.
>          */
>         buf_printf(b, "#include <linux/build-salt.h>\n");
> -       buf_printf(b, "#include <linux/vermagic.h>\n");
> +       buf_printf(b, "#include <../kernel/vermagic.h>\n");


I hate this.


#error can break the build if the header is included in a wrong way.

For example, include/acpi/platform/aclinux.h



I prefer something like this if a big hammer is needed here.

diff --git a/include/linux/vermagic.h b/include/linux/vermagic.h
index 9aced11e9000..d69fa4661715 100644
--- a/include/linux/vermagic.h
+++ b/include/linux/vermagic.h
@@ -1,4 +1,9 @@
 /* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef INCLUDE_VERMAGIC
+#error "This header can be included from kernel/module.c or *.mod.c"
+#endif
+
 #include <generated/utsrelease.h>

 /* Simply sanity version stamp for modules. */
diff --git a/kernel/module.c b/kernel/module.c
index 646f1e2330d2..8833e848b73c 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -4,6 +4,9 @@
    Copyright (C) 2001 Rusty Russell, 2002, 2010 Rusty Russell IBM.

 */
+
+#define INCLUDE_VERMAGIC
+
 #include <linux/export.h>
 #include <linux/extable.h>
 #include <linux/moduleloader.h>
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 5c3c50c5ec52..7f7d4ee7b652 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -2251,6 +2251,7 @@ static void add_header(struct buffer *b, struct
module *mod)
         * Include build-salt.h after module.h in order to
         * inherit the definitions.
         */
+       buf_printf(b, "#define INCLUDE_VERMAGIC\n");
        buf_printf(b, "#include <linux/build-salt.h>\n");
        buf_printf(b, "#include <linux/vermagic.h>\n");
        buf_printf(b, "#include <linux/compiler.h>\n");




-- 
Best Regards
Masahiro Yamada

Re: [PATCH net-next v1 4/4] kernel/module: Hide vermagic header file from general use

[Leon Romanovsky]

On Fri, Apr 17, 2020 at 09:07:10PM +0900, Masahiro Yamada wrote:
> On Wed, Apr 15, 2020 at 10:37 PM Leon Romanovsky <leon@kernel.org> wrote:
> >
> > From: Leon Romanovsky <leonro@mellanox.com>
> >
> > VERMAGIC* definitions are not supposed to be used by the drivers,
> > see this [1] bug report, so simply move this header file to be visible
> > to kernel/* and scripts files only.
> >
> > In-tree module build:
> > ➜  kernel git:(vermagic) ✗ make clean
> > ➜  kernel git:(vermagic) ✗ make M=drivers/infiniband/hw/mlx5
> > ➜  kernel git:(vermagic) ✗ modinfo drivers/infiniband/hw/mlx5/mlx5_ib.ko
> > filename:       /images/leonro/src/kernel/drivers/infiniband/hw/mlx5/mlx5_ib.ko
> > <...>
> > vermagic:       5.6.0+ SMP mod_unload modversions
> >
> > Out-of-tree module build:
> > ➜  mlx5 make -C /images/leonro/src/kernel clean M=/tmp/mlx5
> > ➜  mlx5 make -C /images/leonro/src/kernel M=/tmp/mlx5
> > ➜  mlx5 modinfo /tmp/mlx5/mlx5_ib.ko
> > filename:       /tmp/mlx5/mlx5_ib.ko
> > <...>
> > vermagic:       5.6.0+ SMP mod_unload modversions
> >
> > [1] https://lore.kernel.org/lkml/20200411155623.GA22175@zn.tnic
> > Reported-by: Borislav Petkov <bp@suse.de>
> > Acked-by: Borislav Petkov <bp@suse.de>
> > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> > ---
> >  kernel/module.c                      | 2 +-
> >  {include/linux => kernel}/vermagic.h | 0
> >  scripts/mod/modpost.c                | 2 +-
> >  3 files changed, 2 insertions(+), 2 deletions(-)
> >  rename {include/linux => kernel}/vermagic.h (100%)
> >
> > diff --git a/kernel/module.c b/kernel/module.c
> > index 3447f3b74870..fce06095d341 100644
> > --- a/kernel/module.c
> > +++ b/kernel/module.c
> > @@ -29,7 +29,6 @@
> >  #include <linux/moduleparam.h>
> >  #include <linux/errno.h>
> >  #include <linux/err.h>
> > -#include <linux/vermagic.h>
> >  #include <linux/notifier.h>
> >  #include <linux/sched.h>
> >  #include <linux/device.h>
> > @@ -55,6 +54,7 @@
> >  #include <linux/audit.h>
> >  #include <uapi/linux/module.h>
> >  #include "module-internal.h"
> > +#include "vermagic.h"
> >
> >  #define CREATE_TRACE_POINTS
> >  #include <trace/events/module.h>
> > diff --git a/include/linux/vermagic.h b/kernel/vermagic.h
> > similarity index 100%
> > rename from include/linux/vermagic.h
> > rename to kernel/vermagic.h
> > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> > index 5c3c50c5ec52..91f86261bcfe 100644
> > --- a/scripts/mod/modpost.c
> > +++ b/scripts/mod/modpost.c
> > @@ -2252,7 +2252,7 @@ static void add_header(struct buffer *b, struct module *mod)
> >          * inherit the definitions.
> >          */
> >         buf_printf(b, "#include <linux/build-salt.h>\n");
> > -       buf_printf(b, "#include <linux/vermagic.h>\n");
> > +       buf_printf(b, "#include <../kernel/vermagic.h>\n");
>
>
> I hate this.

Why?

The "ifndef" solution won't achieve the main goal of this patch -
completely disallow usage of vermagic.h. Every kernel developer
knows that headers outside of include/ folder are not meant to be
used.

Once the vermagic.h inside kernel/ folder no one will even try
to include it.

But with new define, all reviewers will need to be aware that they
don't suppose to see it in "regular" patches.

So, instead of making everything clear by being explicit, we will
rely on developers + review + checkpatch and implicit logic.

>
>
> #error can break the build if the header is included in a wrong way.
>
> For example, include/acpi/platform/aclinux.h
>
>
>
> I prefer something like this if a big hammer is needed here.

If it is the only way to progress, I will change the series to use
this variant, however I don't like it both technically and aesthetically
by seeing machinery file with extremely narrow scope inside general include
header.

>
> diff --git a/include/linux/vermagic.h b/include/linux/vermagic.h
> index 9aced11e9000..d69fa4661715 100644
> --- a/include/linux/vermagic.h
> +++ b/include/linux/vermagic.h
> @@ -1,4 +1,9 @@
>  /* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef INCLUDE_VERMAGIC
> +#error "This header can be included from kernel/module.c or *.mod.c"
> +#endif
> +
>  #include <generated/utsrelease.h>
>
>  /* Simply sanity version stamp for modules. */
> diff --git a/kernel/module.c b/kernel/module.c
> index 646f1e2330d2..8833e848b73c 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -4,6 +4,9 @@
>     Copyright (C) 2001 Rusty Russell, 2002, 2010 Rusty Russell IBM.
>
>  */
> +
> +#define INCLUDE_VERMAGIC
> +
>  #include <linux/export.h>
>  #include <linux/extable.h>
>  #include <linux/moduleloader.h>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 5c3c50c5ec52..7f7d4ee7b652 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -2251,6 +2251,7 @@ static void add_header(struct buffer *b, struct
> module *mod)
>          * Include build-salt.h after module.h in order to
>          * inherit the definitions.
>          */
> +       buf_printf(b, "#define INCLUDE_VERMAGIC\n");
>         buf_printf(b, "#include <linux/build-salt.h>\n");
>         buf_printf(b, "#include <linux/vermagic.h>\n");
>         buf_printf(b, "#include <linux/compiler.h>\n");
>
>
>
>
> --
> Best Regards
> Masahiro Yamada