linux-kernel - Re: [PATCH 1/1] misc: Prevent double registration and deregistration of miscdevice

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: <2025082638-parlor-retreat-56ff@gregkh>
Date: Tue, 26 Aug 2025 09:05:37 +0200
From: "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>
To: Xion Wang (王鑫) <Xion.Wang@...iatek.com>
Cc: "linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	wsd_upstream <wsd_upstream@...iatek.com>,
	"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>,
	Huadian Liu (刘华典) <huadian.liu@...iatek.com>,
	"matthias.bgg@...il.com" <matthias.bgg@...il.com>,
	"arnd@...db.de" <arnd@...db.de>,
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
Subject: Re: [PATCH 1/1] misc: Prevent double registration and deregistration
 of miscdevice

On Tue, Aug 26, 2025 at 02:55:59AM +0000, Xion Wang (王鑫) wrote:
> On Mon, 2025-08-25 at 22:28 +0200, Greg Kroah-Hartman wrote:
> > External email : Please do not click links or open attachments until
> > you have verified the sender or the content.
> > 
> > 
> > On Mon, Aug 25, 2025 at 04:45:47PM +0800, xion.wang@...iatek.com
> > wrote:
> > > From: Xion Wang <xion.wang@...iatek.com>
> > > 
> > > When repeated calls to misc_register() or misc_deregister() on the
> > > same miscdevice could lead to kernel crashes or misc_list
> > > corruption due to
> > > multiple INIT_LIST_HEAD or list_del operations on the same list
> > > node.
> > > 
> > > This patch improves the robustness of the misc device driver by
> > > preventing
> > > both double registration and double deregistration of miscdevice
> > > instances.
> > > 
> > > Signed-off-by: Xion Wang <xion.wang@...iatek.com>
> > > ---
> > >  drivers/char/misc.c | 7 ++++++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/char/misc.c b/drivers/char/misc.c
> > > index 558302a64dd9..2f8666312966 100644
> > > --- a/drivers/char/misc.c
> > > +++ b/drivers/char/misc.c
> > > @@ -210,6 +210,9 @@ int misc_register(struct miscdevice *misc)
> > >       int err = 0;
> > >       bool is_dynamic = (misc->minor == MISC_DYNAMIC_MINOR);
> > > 
> > > +     if (WARN_ON(misc->this_device))
> > > +             return -EEXIST;
> > 
> > You just crashed the kernel if this ever triggers (remember when
> > panic-on-warn is set)
> > 
> > So please, if this can happen, properly handle it.
> > 
> > > +
> > >       INIT_LIST_HEAD(&misc->list);
> > > 
> > >       mutex_lock(&misc_mtx);
> > > @@ -251,6 +254,7 @@ int misc_register(struct miscdevice *misc)
> > >                       misc->minor = MISC_DYNAMIC_MINOR;
> > >               }
> > >               err = PTR_ERR(misc->this_device);
> > > +             misc->this_device = NULL;
> > >               goto out;
> > >       }
> > > 
> > > @@ -275,12 +279,13 @@ EXPORT_SYMBOL(misc_register);
> > > 
> > >  void misc_deregister(struct miscdevice *misc)
> > >  {
> > > -     if (WARN_ON(list_empty(&misc->list)))
> > > +     if (WARN_ON(!misc->this_device))
> > >               return;
> > > 
> > >       mutex_lock(&misc_mtx);
> > >       list_del(&misc->list);
> > >       device_destroy(&misc_class, MKDEV(MISC_MAJOR, misc->minor));
> > > +     misc->this_device = NULL;
> > 
> > You are overloading the pointer here to mean something, please don't.
> > 
> > Again, why would this ever happen?  What in-tree driver does this?
> > 
> > thanks,
> > 
> > greg k-h
> 
> 
> This issue was encountered during MTK internal stress testing,
> specifically in the WiFi module on/off process. If the WiFi module
> fails during the "on" process, it triggers an "off" process. However,
> if the "off" process also fails, the module may not be properly
> deinitialized, and the misc device may not be correctly deregistered.
> On the next WiFi "on" attempt, repeated registration of the misc device
> leads to corruption of the misc_list. Subsequently, when a device calls
> misc_open, it may acquire the misc_lock and enter an infinite loop in
> list_for_each_entry due to the corrupted list, while other threads
> attempting to access their misc device nodes become blocked waiting for
> the misc_lock.

What driver is this?  And wifi devices should be using the rfkill api,
which only registers a misc device at module load time.  A wifi driver
should not be creating a miscdevice itself, that feels very very wrong.

> This scenario exposes two issues:
> 
> Incomplete failure handling in our internal WiFi module's on/off
> process (which we have already addressed internally).
> The lack of a mechanism in the miscdevice framework to prevent repeated
> registration or deregistration, which would improve its robustness.

Again, this shouldn't be something that any driver should hit as this
usage is not in the kernel tree that I can see.  Attempting to
re-register a device multiple times is normally never a good idea.

thanks,

greg k-h