linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/1] misc: Prevent double registration and deregistration of miscdevice
@ 2025-08-25  8:45 xion.wang
  2025-08-25  8:45 ` [PATCH 1/1] " xion.wang
  2025-08-25 20:27 ` [PATCH 0/1] " Greg Kroah-Hartman
  0 siblings, 2 replies; 11+ messages in thread
From: xion.wang @ 2025-08-25  8:45 UTC (permalink / raw)
  To: Arnd Bergmann, Greg Kroah-Hartman, Matthias Brugger,
	AngeloGioacchino Del Regno
  Cc: wsd_upstream, huadian.liu, Xion Wang, linux-kernel,
	linux-arm-kernel, linux-mediatek

From: Xion Wang <xion.wang@mediatek.com>

Dear maintainers,

I am submitting a patch to improve the robustness of the misc device subsystem in the Linux kernel.

In the current implementation, repeated calls to misc_register() or misc_deregister() on the same miscdevice instance may result in corruption of the misc_list or kernel crash due to multiple INIT_LIST_HEAD or list_del operations on the same list node.

This patch introduces additional checks in both misc_register() and misc_deregister() to prevent double registration and double deregistration. By using misc->this_device as a status flag, the driver can safely determine whether the device has already been registered or deregistered, and avoid performing dangerous operations on the misc_list.

With these changes, the misc device subsystem becomes more stable and reliable, reducing the risks of list corruption and improving overall system safety.

Feedback and suggestions are welcome.

Thank you for your time and consideration.

Best regards,
Xion Wang

Xion Wang (1):
  misc: Prevent double registration and deregistration of miscdevice

 drivers/char/misc.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

-- 
2.45.2



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/1] misc: Prevent double registration and deregistration of miscdevice
  2025-08-25  8:45 [PATCH 0/1] misc: Prevent double registration and deregistration of miscdevice xion.wang
@ 2025-08-25  8:45 ` xion.wang
  2025-08-25 20:28   ` Greg Kroah-Hartman
  2025-08-25 20:27 ` [PATCH 0/1] " Greg Kroah-Hartman
  1 sibling, 1 reply; 11+ messages in thread
From: xion.wang @ 2025-08-25  8:45 UTC (permalink / raw)
  To: Arnd Bergmann, Greg Kroah-Hartman, Matthias Brugger,
	AngeloGioacchino Del Regno
  Cc: wsd_upstream, huadian.liu, Xion Wang, linux-kernel,
	linux-arm-kernel, linux-mediatek

From: Xion Wang <xion.wang@mediatek.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@mediatek.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;
+
 	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;
 	misc_minor_free(misc->minor);
 	mutex_unlock(&misc_mtx);
 }
-- 
2.45.2



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/1] misc: Prevent double registration and deregistration of miscdevice
  2025-08-25  8:45 [PATCH 0/1] misc: Prevent double registration and deregistration of miscdevice xion.wang
  2025-08-25  8:45 ` [PATCH 1/1] " xion.wang
@ 2025-08-25 20:27 ` Greg Kroah-Hartman
  1 sibling, 0 replies; 11+ messages in thread
From: Greg Kroah-Hartman @ 2025-08-25 20:27 UTC (permalink / raw)
  To: xion.wang
  Cc: Arnd Bergmann, Matthias Brugger, AngeloGioacchino Del Regno,
	wsd_upstream, huadian.liu, linux-kernel, linux-arm-kernel,
	linux-mediatek

On Mon, Aug 25, 2025 at 04:45:46PM +0800, xion.wang@mediatek.com wrote:
> From: Xion Wang <xion.wang@mediatek.com>
> 
> Dear maintainers,
> 
> I am submitting a patch to improve the robustness of the misc device subsystem in the Linux kernel.
> 
> In the current implementation, repeated calls to misc_register() or misc_deregister() on the same miscdevice instance may result in corruption of the misc_list or kernel crash due to multiple INIT_LIST_HEAD or list_del operations on the same list node.

Don't do that then :)

Seriously, what in-tree driver does that?

> This patch introduces additional checks in both misc_register() and misc_deregister() to prevent double registration and double deregistration. By using misc->this_device as a status flag, the driver can safely determine whether the device has already been registered or deregistered, and avoid performing dangerous operations on the misc_list.
> 
> With these changes, the misc device subsystem becomes more stable and reliable, reducing the risks of list corruption and improving overall system safety.

But again, what driver is doing this?  Why is this really needed?

thanks,

greg k-h


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/1] misc: Prevent double registration and deregistration of miscdevice
  2025-08-25  8:45 ` [PATCH 1/1] " xion.wang
@ 2025-08-25 20:28   ` Greg Kroah-Hartman
  2025-08-26  2:55     ` Xion Wang (王鑫)
  0 siblings, 1 reply; 11+ messages in thread
From: Greg Kroah-Hartman @ 2025-08-25 20:28 UTC (permalink / raw)
  To: xion.wang
  Cc: Arnd Bergmann, Matthias Brugger, AngeloGioacchino Del Regno,
	wsd_upstream, huadian.liu, linux-kernel, linux-arm-kernel,
	linux-mediatek

On Mon, Aug 25, 2025 at 04:45:47PM +0800, xion.wang@mediatek.com wrote:
> From: Xion Wang <xion.wang@mediatek.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@mediatek.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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/1] misc: Prevent double registration and deregistration of miscdevice
  2025-08-25 20:28   ` Greg Kroah-Hartman
@ 2025-08-26  2:55     ` Xion Wang (王鑫)
  2025-08-26  7:05       ` gregkh
  0 siblings, 1 reply; 11+ messages in thread
From: Xion Wang (王鑫) @ 2025-08-26  2:55 UTC (permalink / raw)
  To: gregkh@linuxfoundation.org
  Cc: linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org,
	wsd_upstream, linux-arm-kernel@lists.infradead.org,
	Huadian Liu (刘华典), matthias.bgg@gmail.com,
	arnd@arndb.de, AngeloGioacchino Del Regno

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@mediatek.com
> wrote:
> > From: Xion Wang <xion.wang@mediatek.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@mediatek.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.

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.

During our internal code review, we found that misc_deregister() uses
list_empty to check if misc->list is empty, but this does not
effectively prevent cases where a misc_device is not registered or is
deregistered multiple times. This can result in invalid list_del
operations and trigger a kernel panic. 

Regarding your feedback, you are absolutely right. overloading the
this_device pointer as a registration state flag is not ideal. I will
revise the patch to introduce a dedicated boolean flag in struct
miscdevice to explicitly track the registration state.

We appreciate any further feedback or suggestions from the community to
help improve this solution.

thanks,

xion wang

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/1] misc: Prevent double registration and deregistration of miscdevice
  2025-08-26  2:55     ` Xion Wang (王鑫)
@ 2025-08-26  7:05       ` gregkh
  2025-08-26  7:58         ` Xion Wang (王鑫)
  0 siblings, 1 reply; 11+ messages in thread
From: gregkh @ 2025-08-26  7:05 UTC (permalink / raw)
  To: Xion Wang (王鑫)
  Cc: linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org,
	wsd_upstream, linux-arm-kernel@lists.infradead.org,
	Huadian Liu (刘华典), matthias.bgg@gmail.com,
	arnd@arndb.de, AngeloGioacchino Del Regno

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@mediatek.com
> > wrote:
> > > From: Xion Wang <xion.wang@mediatek.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@mediatek.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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/1] misc: Prevent double registration and deregistration of miscdevice
  2025-08-26  7:05       ` gregkh
@ 2025-08-26  7:58         ` Xion Wang (王鑫)
  2025-08-26 10:40           ` gregkh
  0 siblings, 1 reply; 11+ messages in thread
From: Xion Wang (王鑫) @ 2025-08-26  7:58 UTC (permalink / raw)
  To: gregkh@linuxfoundation.org
  Cc: linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
	wsd_upstream, linux-arm-kernel@lists.infradead.org,
	Huadian Liu (刘华典), matthias.bgg@gmail.com,
	arnd@arndb.de, AngeloGioacchino Del Regno

On Tue, 2025-08-26 at 09:05 +0200, gregkh@linuxfoundation.org wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> 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@mediatek.com
> > > wrote:
> > > > From: Xion Wang <xion.wang@mediatek.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@mediatek.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.
> 
> 

Thank you for your comments.

I am not the owner of the WiFi driver and do not have full details of
its internal logic. However, during internal integration and stress
testing, we observed an issue where repeated registration and
deregistration of a misc device by the WiFi module led to corruption of
the misc_list. While I cannot provide the exact reasoning behind the
WiFi driver's design, I wanted to report the problem and share our
findings with the community in case similar patterns exist elsewhere,
including in vendor or out-of-tree drivers.

Below is a real log excerpt showing the kernel panic caused by repeated
misc device registration:


[21998.088014].(7)[T770  ] mtk_wland_threa:
[wlan][770]wlanOnAtReset:(INIT DEBUG) reset success
[22001.380742].(4)[T776  ] wlan_rst_thread: [WIFI-FW]
mtk_wcn_wlan_func_ctrl[E]: WiFi on/off takes more than 15 seconds
[22001.382096].(4)[T776  ] wlan_rst_thread: [MTK-WIFI]
wifi_reset_end[E]: WMT turn on WIFI fail!

[22002.194944].(4)[T770  ] mtk_wland_threa:
[wlan][770]kalIoctlByBssIdx:(OID WARN) Driver is resetting.
[22002.194949].(4)[T770  ] mtk_wland_threa:
[wlan][770]wlanOnAtReset:(REQ WARN) disassociate error:c0010011
[22002.194953].(4)[T770  ] mtk_wland_threa:
[wlan][770]hifAxiProbe:(INIT ERROR) pfWlanProbe fail!
[22002.194958].(4)[T770  ] mtk_wland_threa:
[wlan][770]wlanFuncOnImpl:(HAL ERROR) glBusFuncOn failed, ret=-1

[22002.970751].(5)[T776  ] wlan_rst_thread:
[wlan][776]glResetMsgHandler:(INIT WARN) WF chip reset start!
[22002.985825].(6)[T776  ] wlan_rst_thread:
[wlan][776]glResetMsgHandler:(INIT WARN) WF chip reset end!
[22002.990568].(6)[T776  ] wlan_rst_thread: [MTK-WIFI]
wifi_reset_end[W]: WIFI is already off.
[22016.162096].(4)[T776  ] wlan_rst_thread: [MTK-WIFI]
wifi_reset_end[W]: WIFI state recovering...
[22016.163209].(4)[T776  ] wlan_rst_thread: [MTK-WIFI]
wifi_reset_end[W]: WIFI is already off.

[22025.798095].(7)[T4045 ] binder:4037_3: [MTK-WIFI] WIFI_write[I]:
WIFI_write 1, length 2, copy_size 2
[22025.827948].(5)[T770  ] mtk_wland_threa: [wlan][770]wlanProbe:(INIT
DEBUG) enter wlanProbe

[22053.185845].(6)[T770  ] mtk_wland_threa: sysfs: cannot create
duplicate filename '/devices/virtual/misc/wlan'
[22053.187114].(6)[T770  ] mtk_wland_threa: CPU: 6 UID: 0 PID: 770
Comm: mtk_wland_threa Tainted: G           OE      6.12.30-android16-5-
gc0222621e3b8-4k #1 c67b8daf991e21c8488e3ba1448fdac319baaf99
[22053.187118].(6)[T770  ] mtk_wland_threa: Tainted: [O]=OOT_MODULE,
[E]=UNSIGNED_MODULE
[22053.187119].(6)[T770  ] mtk_wland_threa: Hardware name: MT6858(ENG)
(DT)
[22053.187121].(6)[T770  ] mtk_wland_threa: Call trace:
[22053.187122].(6)[T770  ] mtk_wland_threa:  dump_backtrace+0xf8/0x178
[22053.187130].(6)[T770  ] mtk_wland_threa:  show_stack+0x18/0x28
[22053.187133].(6)[T770  ] mtk_wland_threa:  dump_stack_lvl+0x40/0x88
[22053.187136].(6)[T770  ] mtk_wland_threa:  dump_stack+0x18/0x24
[22053.187139].(6)[T770  ] mtk_wland_threa:  sysfs_warn_dup+0x6c/0xc8
[22053.187143].(6)[T770  ]
mtk_wland_threa:  sysfs_create_dir_ns+0xa0/0xf8
[22053.187145].(6)[T770  ]
mtk_wland_threa:  kobject_add_internal+0x1b8/0x47c
[22053.187148].(6)[T770  ] mtk_wland_threa:  kobject_add+0x90/0x104
[22053.187151].(6)[T770  ] mtk_wland_threa:  device_add+0x1a4/0x494
[22053.187154].(6)[T770  ]
mtk_wland_threa:  device_create_groups_vargs+0xd4/0x168
[22053.187156].(6)[T770  ]
mtk_wland_threa:  device_create_with_groups+0x48/0x74
[22053.187158].(6)[T770  ] mtk_wland_threa:  misc_register+0x1b8/0x28c
[22053.187160].(6)[T770  ]
mtk_wland_threa:  kalWlanUeventInit+0x3c/0xfc [wlan_drv_gen4m_6858]
[22053.187864].(6)[T770  ] mtk_wland_threa:  wlanProbe+0x60c/0x830
[wlan_drv_gen4m_6858]
[22053.188556].(6)[T770  ] mtk_wland_threa:  glBusFuncOn+0x50/0xfc
[wlan_drv_gen4m_6858]
[22053.189246].(6)[T770  ] mtk_wland_threa:  wlanFuncOnImpl+0x78/0x140
[wlan_drv_gen4m_6858]
[22053.189934].(6)[T770  ] mtk_wland_threa:  wlanFuncOn+0x6c/0x128
[wlan_drv_gen4m_6858]
[22053.190622].(6)[T770  ]
mtk_wland_threa:  wlan_func_on_by_chrdev+0x98/0x108
[wlan_drv_gen4m_6858]
[22053.191307].(6)[T770  ]
mtk_wland_threa:  mtk_wland_thread_main+0x98/0x334
[wmt_chrdev_wifi_connac2]
[22053.191318].(6)[T770  ] mtk_wland_threa:  kthread+0x110/0x1a4
[22053.191321].(6)[T770  ] mtk_wland_threa:  ret_from_fork+0x10/0x20
[22053.218071].(6)[T770  ] mtk_wland_threa: kobject:
kobject_add_internal failed for wlan with -EEXIST, don't try to
register things with the same name in the same directory.
[22053.220025].(6)[T770  ] mtk_wland_threa:
[wlan][770]kalWlanUeventInit:(INIT WARN) misc_register error:-17


I understand that this usage is not typical for in-tree drivers, and
that robust failure handling is expected. 

Regarding the robustness check in misc_deregister(), the current
WARN_ON(list_empty(&misc->list)) does not fully prevent errors such as
deregistering an unregistered or already deregistered device. If the
community feels such checks are unnecessary, I am open to removing or
revising this logic as advised.

Thank you again for your guidance and feedback.



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/1] misc: Prevent double registration and deregistration of miscdevice
  2025-08-26  7:58         ` Xion Wang (王鑫)
@ 2025-08-26 10:40           ` gregkh
  2025-08-26 12:09             ` Xion Wang (王鑫)
  0 siblings, 1 reply; 11+ messages in thread
From: gregkh @ 2025-08-26 10:40 UTC (permalink / raw)
  To: Xion Wang (王鑫)
  Cc: linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
	wsd_upstream, linux-arm-kernel@lists.infradead.org,
	Huadian Liu (刘华典), matthias.bgg@gmail.com,
	arnd@arndb.de, AngeloGioacchino Del Regno

On Tue, Aug 26, 2025 at 07:58:47AM +0000, Xion Wang (王鑫) wrote:
> > 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.
> 
> Thank you for your comments.
> 
> I am not the owner of the WiFi driver and do not have full details of
> its internal logic. However, during internal integration and stress
> testing, we observed an issue where repeated registration and
> deregistration of a misc device by the WiFi module led to corruption of
> the misc_list. While I cannot provide the exact reasoning behind the
> WiFi driver's design, I wanted to report the problem and share our
> findings with the community in case similar patterns exist elsewhere,
> including in vendor or out-of-tree drivers.

We do not "harden" our internal apis for external drivers, we fix
drivers to not do foolish things :)

Please fix your out-of-tree code, it should not be even touching the
miscdev api, as that is not something a wifi driver should be
interacting with.  Please use the correct one instead, and then you will
not have this type of issue.

thanks,

greg k-h


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/1] misc: Prevent double registration and deregistration of miscdevice
  2025-08-26 10:40           ` gregkh
@ 2025-08-26 12:09             ` Xion Wang (王鑫)
  2025-08-26 12:54               ` gregkh
  0 siblings, 1 reply; 11+ messages in thread
From: Xion Wang (王鑫) @ 2025-08-26 12:09 UTC (permalink / raw)
  To: gregkh@linuxfoundation.org
  Cc: linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
	wsd_upstream, linux-arm-kernel@lists.infradead.org,
	Huadian Liu (刘华典), matthias.bgg@gmail.com,
	arnd@arndb.de, AngeloGioacchino Del Regno

On Tue, 2025-08-26 at 12:40 +0200, gregkh@linuxfoundation.org wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On Tue, Aug 26, 2025 at 07:58:47AM +0000, Xion Wang (王鑫) wrote:
> > > 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.
> > 
> > Thank you for your comments.
> > 
> > I am not the owner of the WiFi driver and do not have full details
> > of
> > its internal logic. However, during internal integration and stress
> > testing, we observed an issue where repeated registration and
> > deregistration of a misc device by the WiFi module led to
> > corruption of
> > the misc_list. While I cannot provide the exact reasoning behind
> > the
> > WiFi driver's design, I wanted to report the problem and share our
> > findings with the community in case similar patterns exist
> > elsewhere,
> > including in vendor or out-of-tree drivers.
> 
> We do not "harden" our internal apis for external drivers, we fix
> drivers to not do foolish things :)
> 
> Please fix your out-of-tree code, it should not be even touching the
> miscdev api, as that is not something a wifi driver should be
> interacting with.  Please use the correct one instead, and then you
> will
> not have this type of issue.

Thank you for your feedback.

I agree that the kernel should not be hardened for out-of-tree drivers
misusing internal APIs. We will update our internal code to follow best
practices and avoid improper use of the miscdevice API.

On a related note, the current 'WARN_ON(list_empty(&misc->list))' check
in misc_deregister() does not catch any practical error conditions:

For statically allocated miscdevice structs, the list pointers are
zero-initialized, so list_empty() will return false, not true.
After list_del(), the pointers are set to LIST_POISON1/2, so repeated
deregistration also fails to trigger the check.

Since this condition does not protect in-tree drivers or catch real
errors, would it be reasonable to remove it?

I can submit a patch if the community agrees.

thanks,

xion wang



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/1] misc: Prevent double registration and deregistration of miscdevice
  2025-08-26 12:09             ` Xion Wang (王鑫)
@ 2025-08-26 12:54               ` gregkh
  2025-09-02  1:42                 ` Xion Wang (王鑫)
  0 siblings, 1 reply; 11+ messages in thread
From: gregkh @ 2025-08-26 12:54 UTC (permalink / raw)
  To: Xion Wang (王鑫)
  Cc: linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
	wsd_upstream, linux-arm-kernel@lists.infradead.org,
	Huadian Liu (刘华典), matthias.bgg@gmail.com,
	arnd@arndb.de, AngeloGioacchino Del Regno

On Tue, Aug 26, 2025 at 12:09:01PM +0000, Xion Wang (王鑫) wrote:
> On Tue, 2025-08-26 at 12:40 +0200, gregkh@linuxfoundation.org wrote:
> > External email : Please do not click links or open attachments until
> > you have verified the sender or the content.
> > 
> > 
> > On Tue, Aug 26, 2025 at 07:58:47AM +0000, Xion Wang (王鑫) wrote:
> > > > 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.
> > > 
> > > Thank you for your comments.
> > > 
> > > I am not the owner of the WiFi driver and do not have full details
> > > of
> > > its internal logic. However, during internal integration and stress
> > > testing, we observed an issue where repeated registration and
> > > deregistration of a misc device by the WiFi module led to
> > > corruption of
> > > the misc_list. While I cannot provide the exact reasoning behind
> > > the
> > > WiFi driver's design, I wanted to report the problem and share our
> > > findings with the community in case similar patterns exist
> > > elsewhere,
> > > including in vendor or out-of-tree drivers.
> > 
> > We do not "harden" our internal apis for external drivers, we fix
> > drivers to not do foolish things :)
> > 
> > Please fix your out-of-tree code, it should not be even touching the
> > miscdev api, as that is not something a wifi driver should be
> > interacting with.  Please use the correct one instead, and then you
> > will
> > not have this type of issue.
> 
> Thank you for your feedback.
> 
> I agree that the kernel should not be hardened for out-of-tree drivers
> misusing internal APIs. We will update our internal code to follow best
> practices and avoid improper use of the miscdevice API.
> 
> On a related note, the current 'WARN_ON(list_empty(&misc->list))' check
> in misc_deregister() does not catch any practical error conditions:
> 
> For statically allocated miscdevice structs, the list pointers are
> zero-initialized, so list_empty() will return false, not true.
> After list_del(), the pointers are set to LIST_POISON1/2, so repeated
> deregistration also fails to trigger the check.
> 
> Since this condition does not protect in-tree drivers or catch real
> errors, would it be reasonable to remove it?

Yes, if it can never be hit, we should remove it.

> I can submit a patch if the community agrees.

That would be great, thank you!

greg k-h


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/1] misc: Prevent double registration and deregistration of miscdevice
  2025-08-26 12:54               ` gregkh
@ 2025-09-02  1:42                 ` Xion Wang (王鑫)
  0 siblings, 0 replies; 11+ messages in thread
From: Xion Wang (王鑫) @ 2025-09-02  1:42 UTC (permalink / raw)
  To: gregkh@linuxfoundation.org
  Cc: linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
	wsd_upstream, linux-arm-kernel@lists.infradead.org,
	Huadian Liu (刘华典), matthias.bgg@gmail.com,
	arnd@arndb.de, AngeloGioacchino Del Regno

On Tue, 2025-08-26 at 14:54 +0200, gregkh@linuxfoundation.org wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On Tue, Aug 26, 2025 at 12:09:01PM +0000, Xion Wang (王鑫) wrote:
> > On Tue, 2025-08-26 at 12:40 +0200, gregkh@linuxfoundation.org
> > wrote:
> > > External email : Please do not click links or open attachments
> > > until
> > > you have verified the sender or the content.
> > > 
> > > 
> > > On Tue, Aug 26, 2025 at 07:58:47AM +0000, Xion Wang (王鑫) wrote:
> > > > > 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.
> > > > 
> > > > Thank you for your comments.
> > > > 
> > > > I am not the owner of the WiFi driver and do not have full
> > > > details
> > > > of
> > > > its internal logic. However, during internal integration and
> > > > stress
> > > > testing, we observed an issue where repeated registration and
> > > > deregistration of a misc device by the WiFi module led to
> > > > corruption of
> > > > the misc_list. While I cannot provide the exact reasoning
> > > > behind
> > > > the
> > > > WiFi driver's design, I wanted to report the problem and share
> > > > our
> > > > findings with the community in case similar patterns exist
> > > > elsewhere,
> > > > including in vendor or out-of-tree drivers.
> > > 
> > > We do not "harden" our internal apis for external drivers, we fix
> > > drivers to not do foolish things :)
> > > 
> > > Please fix your out-of-tree code, it should not be even touching
> > > the
> > > miscdev api, as that is not something a wifi driver should be
> > > interacting with.  Please use the correct one instead, and then
> > > you
> > > will
> > > not have this type of issue.
> > 
> > Thank you for your feedback.
> > 
> > I agree that the kernel should not be hardened for out-of-tree
> > drivers
> > misusing internal APIs. We will update our internal code to follow
> > best
> > practices and avoid improper use of the miscdevice API.
> > 
> > On a related note, the current 'WARN_ON(list_empty(&misc->list))'
> > check
> > in misc_deregister() does not catch any practical error conditions:
> > 
> > For statically allocated miscdevice structs, the list pointers are
> > zero-initialized, so list_empty() will return false, not true.
> > After list_del(), the pointers are set to LIST_POISON1/2, so
> > repeated
> > deregistration also fails to trigger the check.
> > 
> > Since this condition does not protect in-tree drivers or catch real
> > errors, would it be reasonable to remove it?
> 
> Yes, if it can never be hit, we should remove it.
> 
> > I can submit a patch if the community agrees.
> 
> That would be great, thank you!
> 

Hi,

Thank you for your previous feedback.

As suggested, I have submitted a new patch to remove the ineffective
WARN_ON() check in misc_deregister() in a separate email thread:

	
https://lore.kernel.org/lkml/20250827024201.21407-1-xion.wang@mediatek.com/T/#t
	[PATCH 1/1] misc: remove ineffective WARN_ON() check from
misc_deregister()

Please let me know if you have any comments.

Thanks,
Xion Wang

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2025-09-02  1:45 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-25  8:45 [PATCH 0/1] misc: Prevent double registration and deregistration of miscdevice xion.wang
2025-08-25  8:45 ` [PATCH 1/1] " xion.wang
2025-08-25 20:28   ` Greg Kroah-Hartman
2025-08-26  2:55     ` Xion Wang (王鑫)
2025-08-26  7:05       ` gregkh
2025-08-26  7:58         ` Xion Wang (王鑫)
2025-08-26 10:40           ` gregkh
2025-08-26 12:09             ` Xion Wang (王鑫)
2025-08-26 12:54               ` gregkh
2025-09-02  1:42                 ` Xion Wang (王鑫)
2025-08-25 20:27 ` [PATCH 0/1] " Greg Kroah-Hartman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).