| Message ID | 20201112004021.834673-1-joel@jms.id.au |
|---|---|
| State | New |
| Headers | show |
| Series | net/ncsi: Fix re-registering ncsi device | expand |
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.
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,
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