net/3com/3c515: Fix MODULE_ARCH_VERMAGIC redefinition

Commit Message

Borislav Petkov April 11, 2020, 3:56 p.m. UTC

From: Borislav Petkov <[e-mail]>

Change the include order so that MODULE_ARCH_VERMAGIC from the arch
header arch/x86/include/asm/module.h gets used instead of the fallback
from include/linux/vermagic.h and thus fix:

  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")
Signed-off-by: Borislav Petkov <[e-mail]>
---
 drivers/net/ethernet/3com/3c515.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Leon Romanovsky April 11, 2020, 4:11 p.m. UTC | #1

On Sat, Apr 11, 2020 at 05:56:23PM +0200, Borislav Petkov wrote:
> From: Borislav Petkov <[e-mail]>
>
> Change the include order so that MODULE_ARCH_VERMAGIC from the arch
> header arch/x86/include/asm/module.h gets used instead of the fallback
> from include/linux/vermagic.h and thus fix:
>
>   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")
> Signed-off-by: Borislav Petkov <[e-mail]>
> ---
>  drivers/net/ethernet/3com/3c515.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Probably, this is the right change, but I have a feeling that the right
solution will be inside headers itself. It is a little bit strange that
both very common kernel headers like module.h and vermagic.h are location
dependant.

Thanks

>
> diff --git a/drivers/net/ethernet/3com/3c515.c b/drivers/net/ethernet/3com/3c515.c
> index 90312fcd6319..cdceef891dbd 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
> @@ -67,6 +66,7 @@ static int max_interrupt_work = 20;
>  #include <linux/timer.h>
>  #include <linux/ethtool.h>
>  #include <linux/bitops.h>
> +#include <linux/vermagic.h>
>
>  #include <linux/uaccess.h>
>  #include <asm/io.h>
> --
> 2.21.0
>
> --
> Regards/Gruss,
>     Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette

Borislav Petkov April 11, 2020, 5:35 p.m. UTC | #2

On Sat, Apr 11, 2020 at 07:11:56PM +0300, Leon Romanovsky wrote:
> Probably, this is the right change, but I have a feeling that the right
> solution will be inside headers itself. It is a little bit strange that
> both very common kernel headers like module.h and vermagic.h are location
> dependant.

Judging by how only a couple of net drivers include vermagic.h directly,
doh, of course:

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

---

Drivers include

#include <linux/module.h>

which includes

#include <asm/module.h>

which defines the arch-specific MODULE_ARCH_VERMAGIC.

Why did you need to include vermagic.h directly? i386 builds fine with
the vermagic.h includes removed or was it some other arches which needed
it?

Thx.

Leon Romanovsky April 11, 2020, 6:10 p.m. UTC | #3

On Sat, Apr 11, 2020 at 07:35:04PM +0200, Borislav Petkov wrote:
> On Sat, Apr 11, 2020 at 07:11:56PM +0300, Leon Romanovsky wrote:
> > Probably, this is the right change, but I have a feeling that the right
> > solution will be inside headers itself. It is a little bit strange that
> > both very common kernel headers like module.h and vermagic.h are location
> > dependant.
>
> Judging by how only a couple of net drivers include vermagic.h directly,
> doh, of course:
>
> 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
>
> ---
>
> Drivers include
>
> #include <linux/module.h>
>
> which includes
>
> #include <asm/module.h>
>
> which defines the arch-specific MODULE_ARCH_VERMAGIC.
>
> Why did you need to include vermagic.h directly? i386 builds fine with
> the vermagic.h includes removed or was it some other arches which needed
> it?

I want to think that it was an outcome of some 0-day kbuild report,
but I am not sure about that anymore [1].

Thanks

[1] https://lists.01.org/hyperkitty/list/[e-mail]/

>
> Thx.
>
> --
> Regards/Gruss,
>     Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette

Borislav Petkov April 11, 2020, 6:41 p.m. UTC | #4

On Sat, Apr 11, 2020 at 09:10:15PM +0300, Leon Romanovsky wrote:
> I want to think that it was an outcome of some 0-day kbuild report,
> but I am not sure about that anymore [1].

I pushed it here:

https://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git/log/?h=rc0%2b0-3c

The 0day bot will send me a mail soon. :)

David Miller April 13, 2020, 4:03 a.m. UTC | #5

From: Borislav Petkov <[e-mail]>
Date: Sat, 11 Apr 2020 17:56:23 +0200

> From: Borislav Petkov <[e-mail]>
> 
> Change the include order so that MODULE_ARCH_VERMAGIC from the arch
> header arch/x86/include/asm/module.h gets used instead of the fallback
> from include/linux/vermagic.h and thus fix:
> 
>   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")
> Signed-off-by: Borislav Petkov <[e-mail]>

I'm so confused, that commit in the Fixes: tag is _removing_ code but adding
new #include directives?!?!

Is vermagic.h really needed in these files?

Leon Romanovsky April 13, 2020, 4:55 a.m. UTC | #6

On Sun, Apr 12, 2020 at 09:03:41PM -0700, David Miller wrote:
> From: Borislav Petkov <[e-mail]>
> Date: Sat, 11 Apr 2020 17:56:23 +0200
>
> > From: Borislav Petkov <[e-mail]>
> >
> > Change the include order so that MODULE_ARCH_VERMAGIC from the arch
> > header arch/x86/include/asm/module.h gets used instead of the fallback
> > from include/linux/vermagic.h and thus fix:
> >
> >   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")
> > Signed-off-by: Borislav Petkov <[e-mail]>
>
> I'm so confused, that commit in the Fixes: tag is _removing_ code but adding
> new #include directives?!?!
>
> Is vermagic.h really needed in these files?

You are completely right, it is not needed at all in those files.

Thanks

David Miller April 13, 2020, 5:07 a.m. UTC | #7

From: Leon Romanovsky <[e-mail]>
Date: Mon, 13 Apr 2020 07:55:55 +0300

> On Sun, Apr 12, 2020 at 09:03:41PM -0700, David Miller wrote:
>> From: Borislav Petkov <[e-mail]>
>> Date: Sat, 11 Apr 2020 17:56:23 +0200
>>
>> > From: Borislav Petkov <[e-mail]>
>> >
>> > Change the include order so that MODULE_ARCH_VERMAGIC from the arch
>> > header arch/x86/include/asm/module.h gets used instead of the fallback
>> > from include/linux/vermagic.h and thus fix:
>> >
>> >   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")
>> > Signed-off-by: Borislav Petkov <[e-mail]>
>>
>> I'm so confused, that commit in the Fixes: tag is _removing_ code but adding
>> new #include directives?!?!
>>
>> Is vermagic.h really needed in these files?
> 
> You are completely right, it is not needed at all in those files.

Ok let's just remove it to fix this.

Leon Romanovsky April 13, 2020, 5:26 a.m. UTC | #8

On Sun, Apr 12, 2020 at 10:07:39PM -0700, David Miller wrote:
> From: Leon Romanovsky <[e-mail]>
> Date: Mon, 13 Apr 2020 07:55:55 +0300
>
> > On Sun, Apr 12, 2020 at 09:03:41PM -0700, David Miller wrote:
> >> From: Borislav Petkov <[e-mail]>
> >> Date: Sat, 11 Apr 2020 17:56:23 +0200
> >>
> >> > From: Borislav Petkov <[e-mail]>
> >> >
> >> > Change the include order so that MODULE_ARCH_VERMAGIC from the arch
> >> > header arch/x86/include/asm/module.h gets used instead of the fallback
> >> > from include/linux/vermagic.h and thus fix:
> >> >
> >> >   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")
> >> > Signed-off-by: Borislav Petkov <[e-mail]>
> >>
> >> I'm so confused, that commit in the Fixes: tag is _removing_ code but adding
> >> new #include directives?!?!
> >>
> >> Is vermagic.h really needed in these files?
> >
> > You are completely right, it is not needed at all in those files.
>
> Ok let's just remove it to fix this.

Thanks a lot.

How do you want us to handle it? Boris resend, me to send, you to fix?

Thanks

David Miller April 13, 2020, 5:36 a.m. UTC | #9

From: Leon Romanovsky <[e-mail]>
Date: Mon, 13 Apr 2020 08:26:37 +0300

> How do you want us to handle it? Boris resend, me to send, you to fix?

Anyone other than me can do it ;-)

Borislav Petkov April 13, 2020, 8:04 a.m. UTC | #10

On Sun, Apr 12, 2020 at 10:36:04PM -0700, David Miller wrote:
> From: Leon Romanovsky <[e-mail]>
> Date: Mon, 13 Apr 2020 08:26:37 +0300
> 
> > How do you want us to handle it? Boris resend, me to send, you to fix?
> 
> Anyone other than me can do it ;-)

Ok, here's what I'm thinking: that vermagic.h is normally automatically
included in the *mod.c as part of the module creation, see add_header()
in modpost.c.

So then perhaps drivers should not use it directly due to the current
inclusion order:

linux/module.h includes asm/module.h and that arch-specific header
defines MODULE_VERMAGIC* for the respective arch.

linux/vermagic.h defines all those fallbacks for those MODULE_VERMAGIC*
things and if the inclusion order is swapped - we get the redefinition
warning.

Yesterday I tried the below - basically get rid of all the remaining
includers of linux/vermagic.h but two are left:

drivers/net/ethernet/hisilicon/hns3/hns3_enet.c:18:#include <linux/vermagic.h>
drivers/net/ethernet/netronome/nfp/nfp_main.c:17:#include <linux/vermagic.h>

because both use VERMAGIC_STRING directly.

So,

* one could either allow that and sort the inclusion order so that, for
example, asm/module.h includes linux/vermagic.h and thus the fallbacks
are there present.

or

* remove all uses of VERMAGIC_STRING from the drivers, add a header
guard which prevents people from using it directly and leave
VERMAGIC_STRING only to the internal module machinery in the kernel.

Judging by how only a handful of old drivers are even using that,
perhaps not too far fetched.

In any case, this needs a maintainer decision.

Leon, if you wanna do it whatever you guys end up agreeing on, just go
ahead and submit the patches - it's not like I don't have anything else
on the TODO :-) Just add a Reported-by: me and that should be enough.

If you're busy too, lemme know and I'll put it on my todo then.

Thx.

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..adab97e500cf 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>
@@ -29,6 +28,8 @@
 #include <net/flow_offload.h>
 #include <linux/ethtool_netlink.h>
 
+#include <generated/utsrelease.h>
+
 #include "common.h"
 
 /*

Leon Romanovsky April 13, 2020, 8:40 a.m. UTC | #11

On Mon, Apr 13, 2020 at 10:04:52AM +0200, Borislav Petkov wrote:
> On Sun, Apr 12, 2020 at 10:36:04PM -0700, David Miller wrote:
> > From: Leon Romanovsky <[e-mail]>
> > Date: Mon, 13 Apr 2020 08:26:37 +0300
> >
> > > How do you want us to handle it? Boris resend, me to send, you to fix?
> >
> > Anyone other than me can do it ;-)
>
> Ok, here's what I'm thinking: that vermagic.h is normally automatically
> included in the *mod.c as part of the module creation, see add_header()
> in modpost.c.
>
> So then perhaps drivers should not use it directly due to the current
> inclusion order:
>
> linux/module.h includes asm/module.h and that arch-specific header
> defines MODULE_VERMAGIC* for the respective arch.
>
> linux/vermagic.h defines all those fallbacks for those MODULE_VERMAGIC*
> things and if the inclusion order is swapped - we get the redefinition
> warning.
>
> Yesterday I tried the below - basically get rid of all the remaining
> includers of linux/vermagic.h but two are left:
>
> drivers/net/ethernet/hisilicon/hns3/hns3_enet.c:18:#include <linux/vermagic.h>
> drivers/net/ethernet/netronome/nfp/nfp_main.c:17:#include <linux/vermagic.h>
>
> because both use VERMAGIC_STRING directly.
>
> So,
>
> * one could either allow that and sort the inclusion order so that, for
> example, asm/module.h includes linux/vermagic.h and thus the fallbacks
> are there present.
>
> or
>
> * remove all uses of VERMAGIC_STRING from the drivers, add a header
> guard which prevents people from using it directly and leave
> VERMAGIC_STRING only to the internal module machinery in the kernel.
>
> Judging by how only a handful of old drivers are even using that,
> perhaps not too far fetched.
>
> In any case, this needs a maintainer decision.
>
> Leon, if you wanna do it whatever you guys end up agreeing on, just go
> ahead and submit the patches - it's not like I don't have anything else
> on the TODO :-) Just add a Reported-by: me and that should be enough.

I broke it so I should fix and will send a patch today/tomorrow.

Thanks

>
> If you're busy too, lemme know and I'll put it on my todo then.
>
> Thx.
>
> 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..adab97e500cf 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>
> @@ -29,6 +28,8 @@
>  #include <net/flow_offload.h>
>  #include <linux/ethtool_netlink.h>
>
> +#include <generated/utsrelease.h>
> +
>  #include "common.h"
>
>  /*
>
>
> --
> Regards/Gruss,
>     Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette

Leon Romanovsky April 14, 2020, 12:23 p.m. UTC | #12

On Mon, Apr 13, 2020 at 11:40:26AM +0300, Leon Romanovsky wrote:
> On Mon, Apr 13, 2020 at 10:04:52AM +0200, Borislav Petkov wrote:
> > On Sun, Apr 12, 2020 at 10:36:04PM -0700, David Miller wrote:
> > > From: Leon Romanovsky <[e-mail]>
> > > Date: Mon, 13 Apr 2020 08:26:37 +0300
> > >
> > > > How do you want us to handle it? Boris resend, me to send, you to fix?
> > >
> > > Anyone other than me can do it ;-)
> >
> > Ok, here's what I'm thinking: that vermagic.h is normally automatically
> > included in the *mod.c as part of the module creation, see add_header()
> > in modpost.c.
> >
> > So then perhaps drivers should not use it directly due to the current
> > inclusion order:
> >
> > linux/module.h includes asm/module.h and that arch-specific header
> > defines MODULE_VERMAGIC* for the respective arch.
> >
> > linux/vermagic.h defines all those fallbacks for those MODULE_VERMAGIC*
> > things and if the inclusion order is swapped - we get the redefinition
> > warning.
> >
> > Yesterday I tried the below - basically get rid of all the remaining
> > includers of linux/vermagic.h but two are left:
> >
> > drivers/net/ethernet/hisilicon/hns3/hns3_enet.c:18:#include <linux/vermagic.h>
> > drivers/net/ethernet/netronome/nfp/nfp_main.c:17:#include <linux/vermagic.h>
> >
> > because both use VERMAGIC_STRING directly.
> >
> > So,
> >
> > * one could either allow that and sort the inclusion order so that, for
> > example, asm/module.h includes linux/vermagic.h and thus the fallbacks
> > are there present.
> >
> > or
> >
> > * remove all uses of VERMAGIC_STRING from the drivers, add a header
> > guard which prevents people from using it directly and leave
> > VERMAGIC_STRING only to the internal module machinery in the kernel.
> >
> > Judging by how only a handful of old drivers are even using that,
> > perhaps not too far fetched.
> >
> > In any case, this needs a maintainer decision.
> >
> > Leon, if you wanna do it whatever you guys end up agreeing on, just go
> > ahead and submit the patches - it's not like I don't have anything else
> > on the TODO :-) Just add a Reported-by: me and that should be enough.
>
> I broke it so I should fix and will send a patch today/tomorrow.

ok, the patches are here, will wait for the kbuild results and will send.
https://git.kernel.org/pub/scm/linux/kernel/git/leon/linux-rdma.git/log/?h=vermagic

Thanks

>
> Thanks
>
> >
> > If you're busy too, lemme know and I'll put it on my todo then.
> >
> > Thx.
> >
> > 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..adab97e500cf 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>
> > @@ -29,6 +28,8 @@
> >  #include <net/flow_offload.h>
> >  #include <linux/ethtool_netlink.h>
> >
> > +#include <generated/utsrelease.h>
> > +
> >  #include "common.h"
> >
> >  /*
> >
> >
> > --
> > Regards/Gruss,
> >     Boris.
> >
> > https://people.kernel.org/tglx/notes-about-netiquette

Patch

diff --git a/drivers/net/ethernet/3com/3c515.c b/drivers/net/ethernet/3com/3c515.c
index 90312fcd6319..cdceef891dbd 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
@@ -67,6 +66,7 @@  static int max_interrupt_work = 20;
 #include <linux/timer.h>
 #include <linux/ethtool.h>
 #include <linux/bitops.h>
+#include <linux/vermagic.h>
 
 #include <linux/uaccess.h>
 #include <asm/io.h>