net/ncsi: Fix re-registering ncsi device

Message ID 20201112004021.834673-1-joel@jms.id.au
State New
Headers show
Series
  • net/ncsi: Fix re-registering ncsi device
Related show

Commit Message

Joel Stanley Nov. 12, 2020, 12:40 a.m.
If a user unbinds and re-binds a ncsi aware driver, the kernel will
attempt to register the netlink interface at runtime. The structure is
marked __ro_after_init so this fails at this point.

 Unable to handle kernel paging request at virtual address 80bb588c
 pgd = 0ff284bb
 [80bb588c] *pgd=80a1941e(bad)
 Internal error: Oops: 80d [#1] SMP ARM
 Modules linked in:
 CPU: 0 PID: 128 Comm: sh Not tainted 5.8.14-00191-g79e816b77665 #223
 Hardware name: Generic DT based system
 PC is at genl_register_family+0x1d4/0x648
 LR is at 0xfe46ffff
 pc : [<807f575c>]    lr : [<fe46ffff>]    psr: 20000013
 sp : b52c9d10  ip : bcc125d4  fp : b52c9d5c
 r10: 8090fedc  r9 : 80bb57a8  r8 : 80bb5894
 r7 : 00000013  r6 : 00000000  r5 : 00000018  r4 : 80bb588c
 r3 : 00000000  r2 : 00000000  r1 : bcc124c0  r0 : 00000018
 Flags: nzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
 Control: 10c5387d  Table: b4dcc06a  DAC: 00000051
 Process sh (pid: 128, stack limit = 0xe54c9aea)

 Backtrace:
 [<807f5588>] (genl_register_family) from [<80913720>] (ncsi_init_netlink+0x20/0x48)
  r10:8090fedc r9:80e6c590 r8:b4db4800 r7:00000000 r6:b52dc478 r5:b4db4800
  r4:b52dc000
 [<80913700>] (ncsi_init_netlink) from [<8090f128>] (ncsi_register_dev+0x1d8/0x238)
  r5:b52dc444 r4:b52dc000
 [<8090ef50>] (ncsi_register_dev) from [<8064f43c>] (ftgmac100_probe+0x6e0/0x838)
  r10:00000004 r9:80a5d898 r8:bd141410 r7:bd140900 r6:bd7f2c84 r5:b4dbdb88
  r4:b4db4800
 [<8064ed5c>] (ftgmac100_probe) from [<805d4d90>] (platform_drv_probe+0x58/0xa8)
  r9:80e527cc r8:00000000 r7:80eb4bcc r6:80e527cc r5:bd141410 r4:00000000

Signed-off-by: Joel Stanley <joel@jms.id.au>

---
 net/ncsi/ncsi-netlink.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.28.0

Comments

Joel Stanley Nov. 12, 2020, 1:30 a.m. | #1
On Thu, 12 Nov 2020 at 01:20, Jakub Kicinski <kuba@kernel.org> wrote:
>

> On Thu, 12 Nov 2020 01:11:04 +0000 Joel Stanley wrote:

> > On Thu, 12 Nov 2020 at 00:57, Jakub Kicinski <kuba@kernel.org> wrote:

> > >

> > > On Thu, 12 Nov 2020 11:10:21 +1030 Joel Stanley wrote:

> > > > If a user unbinds and re-binds a ncsi aware driver, the kernel will

> > > > attempt to register the netlink interface at runtime. The structure is

> > > > marked __ro_after_init so this fails at this point.

> > >

> > > netlink family should be registered at module init and unregistered at

> > > unload. That's a better fix IMO.

> >

> > I don't follow, isn't that what is implemented already?

> >

> > Perhaps I'm getting confused because the systems that use this code

> > build the drivers in. The bug I'm seeing is when we unbind and re-bind

> > the driver without any module loading or unloading.

>

> It's registered from ncsi_register_dev(), which is obviously broken,

> because there is only one family so it would never work if there was

> more than one ncsi netdev.

>

> Looks like NCSI can only be built in, so instead of module init it

> should be a subsys_initcall().

>

> Basically remove ncsi_unregister_netlink(), remove the dev parameter

> from ncsi_init_netlink() and add:

>

> subsys_initcall(ncsi_init_netlink);

>

> at the end of ncsi-netlink.c


Thanks for the explanation, I'll do that.

Patch

diff --git a/net/ncsi/ncsi-netlink.c b/net/ncsi/ncsi-netlink.c
index adddc7707aa4..1641ecbb5b1f 100644
--- a/net/ncsi/ncsi-netlink.c
+++ b/net/ncsi/ncsi-netlink.c
@@ -756,7 +756,7 @@  static const struct genl_small_ops ncsi_ops[] = {
 	},
 };
 
-static struct genl_family ncsi_genl_family __ro_after_init = {
+static struct genl_family ncsi_genl_family = {
 	.name = "NCSI",
 	.version = 0,
 	.maxattr = NCSI_ATTR_MAX,