[BUG] r8188eu: Potential deadlocks in rtw_wx_set_wap/essid functions

[BUG] r8188eu: Potential deadlocks in rtw_wx_set_wap/essid functions

[Gui-Dong Han]

Hello maintainers,

I would like to report a potential lock ordering issue in the r8188eu
driver. This may lead to deadlocks under certain conditions.

The functions rtw_wx_set_wap() and rtw_wx_set_essid() acquire locks in
an order that contradicts the established locking hierarchy observed
in other parts of the driver:

1. They first take &pmlmepriv->scanned_queue.lock
2. Then call rtw_set_802_11_infrastructure_mode() which takes &pmlmepriv->lock

This is inverted compared to the common pattern seen in functions like
rtw_joinbss_event_prehandle(), rtw_createbss_cmd_callback(), and
others, which typically:

1. Take &pmlmepriv->lock first
2. Then take &pmlmepriv->scanned_queue.lock

This lock inversion creates a potential deadlock scenario when these
code paths execute concurrently.

Moreover, the call chain: rtw_wx_set_* ->
rtw_set_802_11_infrastructure_mode() -> rtw_free_assoc_resources()
could lead to recursive acquisition of &pmlmepriv->scanned_queue.lock,
potentially causing self-deadlock even without concurrency.

This issue exists in longterm kernels containing the r8188eu driver:

5.4.y (until 5.4.290)
5.10.y (until 5.10.234)
5.15.y (until 5.15.178)
6.1.y (until 6.1.129)

The r8188eu driver has been removed from upstream, but older
maintained versions (5.4.x–6.1.x) still include this driver and are
affected.

This issue was identified through static analysis. While I've verified
the locking patterns through code review, I'm not sufficiently
familiar with the driver's internals to propose a safe fix.

Thank you for your attention to this matter.

Best regards,
Gui-Dong Han

Re: [BUG] r8188eu: Potential deadlocks in rtw_wx_set_wap/essid functions

[Phillip Potter]

On Tue, Feb 25, 2025 at 09:02:00PM +0800, Gui-Dong Han wrote:
> Hello maintainers,
> 
> I would like to report a potential lock ordering issue in the r8188eu
> driver. This may lead to deadlocks under certain conditions.
> 
> The functions rtw_wx_set_wap() and rtw_wx_set_essid() acquire locks in
> an order that contradicts the established locking hierarchy observed
> in other parts of the driver:
> 
> 1. They first take &pmlmepriv->scanned_queue.lock
> 2. Then call rtw_set_802_11_infrastructure_mode() which takes &pmlmepriv->lock
> 
> This is inverted compared to the common pattern seen in functions like
> rtw_joinbss_event_prehandle(), rtw_createbss_cmd_callback(), and
> others, which typically:
> 
> 1. Take &pmlmepriv->lock first
> 2. Then take &pmlmepriv->scanned_queue.lock
> 
> This lock inversion creates a potential deadlock scenario when these
> code paths execute concurrently.
> 
> Moreover, the call chain: rtw_wx_set_* ->
> rtw_set_802_11_infrastructure_mode() -> rtw_free_assoc_resources()
> could lead to recursive acquisition of &pmlmepriv->scanned_queue.lock,
> potentially causing self-deadlock even without concurrency.
> 
> This issue exists in longterm kernels containing the r8188eu driver:
> 
> 5.4.y (until 5.4.290)
> 5.10.y (until 5.10.234)
> 5.15.y (until 5.15.178)
> 6.1.y (until 6.1.129)
> 
> The r8188eu driver has been removed from upstream, but older
> maintained versions (5.4.x–6.1.x) still include this driver and are
> affected.
> 
> This issue was identified through static analysis. While I've verified
> the locking patterns through code review, I'm not sufficiently
> familiar with the driver's internals to propose a safe fix.
> 
> Thank you for your attention to this matter.
> 
> Best regards,
> Gui-Dong Han

Dear Gui-Dong,

Not sure what the responsibility is here with this driver, given it
never left staging. I've not looked at it myself for years, so genuine
question on my part as to who is responsible for patching it (if at
all). It doesn't have a maintainer anymore to my knowledge.

Also, apologies to be the bearer of bad news, but it upsets me to have
to report Larry Finger sadly passed on last year and is no longer with
us.

Regards,
Phil Potter

Re: [BUG] r8188eu: Potential deadlocks in rtw_wx_set_wap/essid functions

[Dan Carpenter]

On Tue, Feb 25, 2025 at 01:57:05PM +0000, Phillip Potter wrote:
> > This issue exists in longterm kernels containing the r8188eu driver:
> > 
> > 5.4.y (until 5.4.290)
> > 5.10.y (until 5.10.234)
> > 5.15.y (until 5.15.178)
> > 6.1.y (until 6.1.129)

This work is interesting, but we're not going to patch old kernels to
fix static checker bugs.  If the driver were still present in current
kernels or if it were a privilege escalation bug that would be a
different story.

regards,
dan carpenter