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

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

[Leon Romanovsky]

From: Leon Romanovsky <leonro@mellanox.com>

Changelog:
v2:
 * Changed the implementation of patch #4 to be like Masahiro wants.
I personally don't like this implementation and changing it just to move forward
this this patchset.
v1:
https://lore.kernel.org/lkml/20200415133648.1306956-1-leon@kernel.org
 * 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 +-
 include/linux/vermagic.h                             | 5 +++++
 kernel/module.c                                      | 3 +++
 net/ethtool/ioctl.c                                  | 3 +--
 scripts/mod/modpost.c                                | 1 +
 15 files changed, 13 insertions(+), 24 deletions(-)

--
2.25.2

[PATCH net-next v2 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 v2 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 v2 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

Re: [oss-drivers] [PATCH net-next v2 3/4] net/nfp: Update driver to use global kernel version

[Simon Horman]

On Sun, Apr 19, 2020 at 05:18:49PM +0300, Leon Romanovsky wrote:
> 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>

Reviewed-by: Simon Horman <simon.horman@netronome.com>

[PATCH net-next v2 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 introduce special define to guard inclusion
of this header file and define it in kernel/modules.h and in internal
script that generates *.mod.c files.

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>
Acked-by: Jessica Yu <jeyu@kernel.org>
Co-developed-by: Masahiro Yamada <masahiroy@kernel.org>
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 include/linux/vermagic.h | 5 +++++
 kernel/module.c          | 3 +++
 scripts/mod/modpost.c    | 1 +
 3 files changed, 9 insertions(+)

diff --git a/include/linux/vermagic.h b/include/linux/vermagic.h
index 9aced11e9000..7768d20ada39 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 only"
+#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");
--
2.25.2

[PATCH net-next v2 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 introduce special define to guard inclusion
of this header file and define it in kernel/modules.h and in internal
script that generates *.mod.c files.

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>
Acked-by: Jessica Yu <jeyu@kernel.org>
Co-developed-by: Masahiro Yamada <masahiroy@kernel.org>
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 include/linux/vermagic.h | 5 +++++
 kernel/module.c          | 3 +++
 scripts/mod/modpost.c    | 1 +
 3 files changed, 9 insertions(+)

diff --git a/include/linux/vermagic.h b/include/linux/vermagic.h
index 9aced11e9000..7768d20ada39 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 only"
+#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");
--
2.25.2

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

[Masahiro Yamada]

Hi,

On Mon, Apr 20, 2020 at 12:55 AM 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 introduce special define to guard inclusion
> of this header file and define it in kernel/modules.h and in internal
> script that generates *.mod.c files.
>
> 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>
> Acked-by: Jessica Yu <jeyu@kernel.org>
> Co-developed-by: Masahiro Yamada <masahiroy@kernel.org>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> ---


I did not read the full thread of [1],
and perhaps may miss something.

But, this series is trying to solve a different problem
"driver code should not include <linux/vermagic.h>"
isn't it?


IIUC, Borislav reported conflict of MODULE_ARCH_VERMAGIC
if <linux/vermagic.h> is included before <linux/module.h>.

With your cleanups, the include site of <linux/vermagic.h>
will be limited to kernel/module.c and scripts/mod/module.c

Assuming those two files include them in the *correct* order,
this problem will be suppressed.

But, I do not think it addresses the problem properly.


If
  #include <foo.h>
  #include <bar.h>

works, but

  #include <bar.h>
  #include <foo.h>

does not, the root cause is very likely
that <bar.h> is not self-contained.
The problem is solved by including <foo.h> from <bar.h>


Please see my thoughts in this:
https://lore.kernel.org/patchwork/patch/1227024/


Of course, we are solving different issues, so I think
we can merge both.


What do you think?




>  include/linux/vermagic.h | 5 +++++
>  kernel/module.c          | 3 +++
>  scripts/mod/modpost.c    | 1 +
>  3 files changed, 9 insertions(+)
>
> diff --git a/include/linux/vermagic.h b/include/linux/vermagic.h
> index 9aced11e9000..7768d20ada39 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 only"
> +#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");
> --
> 2.25.2
>


--
Best Regards
Masahiro Yamada

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

[Leon Romanovsky]

On Mon, Apr 20, 2020 at 07:59:35AM +0900, Masahiro Yamada wrote:
> Hi,
>
> On Mon, Apr 20, 2020 at 12:55 AM 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 introduce special define to guard inclusion
> > of this header file and define it in kernel/modules.h and in internal
> > script that generates *.mod.c files.
> >
> > 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>
> > Acked-by: Jessica Yu <jeyu@kernel.org>
> > Co-developed-by: Masahiro Yamada <masahiroy@kernel.org>
> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> > ---
>
>
> I did not read the full thread of [1],
> and perhaps may miss something.
>
> But, this series is trying to solve a different problem
> "driver code should not include <linux/vermagic.h>"
> isn't it?
>
>
> IIUC, Borislav reported conflict of MODULE_ARCH_VERMAGIC
> if <linux/vermagic.h> is included before <linux/module.h>.
>
> With your cleanups, the include site of <linux/vermagic.h>
> will be limited to kernel/module.c and scripts/mod/module.c
>
> Assuming those two files include them in the *correct* order,
> this problem will be suppressed.
>
> But, I do not think it addresses the problem properly.
>
>
> If
>   #include <foo.h>
>   #include <bar.h>
>
> works, but
>
>   #include <bar.h>
>   #include <foo.h>
>
> does not, the root cause is very likely
> that <bar.h> is not self-contained.
> The problem is solved by including <foo.h> from <bar.h>
>
>
> Please see my thoughts in this:
> https://lore.kernel.org/patchwork/patch/1227024/
>
>
> Of course, we are solving different issues, so I think
> we can merge both.
>
>
> What do you think?

The idea and rationale are right, include order should not be important.

Thanks

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

[Leon Romanovsky]

On Sun, Apr 19, 2020 at 05:18:46PM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
>
> Changelog:
> v2:
>  * Changed the implementation of patch #4 to be like Masahiro wants.
> I personally don't like this implementation and changing it just to move forward
> this this patchset.
> v1:
> https://lore.kernel.org/lkml/20200415133648.1306956-1-leon@kernel.org
>  * 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
> ----------------------------------------------------------------------------

Dave,

I see in the patchworks that this series is marked as "Needs Review/ACK".
Can you please help me to understand who is needed to be approached?

https://patchwork.ozlabs.org/project/netdev/list/?series=171189

Thanks

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

[David Miller]

From: Leon Romanovsky <leon@kernel.org>
Date: Sun, 19 Apr 2020 17:18:46 +0300

> 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

Series applied to net-next, thanks.