netdev - Re: [PATCH] net/ncsi: Fix re-registering ncsi device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20201111172051.28e70e7b@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Wed, 11 Nov 2020 17:20:51 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Joel Stanley <joel@....id.au>
Cc:     Samuel Mendoza-Jonas <sam@...dozajonas.com>,
        "David S . Miller" <davem@...emloft.net>,
        Andrew Jeffery <andrew@...id.au>,
        Networking <netdev@...r.kernel.org>
Subject: Re: [PATCH] net/ncsi: Fix re-registering ncsi device

On Thu, 12 Nov 2020 01:11:04 +0000 Joel Stanley wrote:
> On Thu, 12 Nov 2020 at 00:57, Jakub Kicinski <kuba@...nel.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