Fixes potential speculative cpu oob read in os_intfs.c by guarding the
index with array_index_nospec.

Fixes smatch warning:
warn: potential spectre issue 'rtw_1d_to_queue' [r]

Signed-off-by: Linus Probert <linus.probert@gmail.com>
---

I can't argue with 100% certainty if this is a real risk. I found the
warning when I was testing out smatch (awesome work on that by the way)
and thought I'd take a look.

I did a quick search on the linux-staging list but couldn't find any
mention of 'spectre'. So as far as I could see it hasn't been brought up
before.

Was unsure if I should prefix this with RFC first.

 drivers/staging/rtl8723bs/os_dep/os_intfs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/os_intfs.c b/drivers/staging/rtl8723bs/os_dep/os_intfs.c
index e943dcea1a21..8eea05111e79 100644
--- a/drivers/staging/rtl8723bs/os_dep/os_intfs.c
+++ b/drivers/staging/rtl8723bs/os_dep/os_intfs.c
@@ -347,7 +347,7 @@ static u16 rtw_select_queue(struct net_device *dev, struct sk_buff *skb,
 	if (pmlmepriv->acm_mask != 0)
 		skb->priority = qos_acm(pmlmepriv->acm_mask, skb->priority);
 
-	return rtw_1d_to_queue[skb->priority];
+	return rtw_1d_to_queue[array_index_nospec(skb->priority, ARRAY_SIZE(rtw_1d_to_queue))];
 }
 
 u16 rtw_recv_select_queue(struct sk_buff *skb)
@@ -374,7 +374,7 @@ u16 rtw_recv_select_queue(struct sk_buff *skb)
 		priority = 0;
 	}
 
-	return rtw_1d_to_queue[priority];
+	return rtw_1d_to_queue[array_index_nospec(priority, ARRAY_SIZE(rtw_1d_to_queue))];
 }
 
 static int rtw_ndev_init(struct net_device *dev)
-- 
2.54.0

On Wed, Apr 29, 2026 at 01:10:16PM +0200, Linus Probert wrote:
> Fixes potential speculative cpu oob read in os_intfs.c by guarding the
> index with array_index_nospec.
> 
> Fixes smatch warning:
> warn: potential spectre issue 'rtw_1d_to_queue' [r]

Is this value controlled by a user?  Or is it just a normal operation
that happens that is not controlled?  In other words, can a user
manipulate this directly to be out of range?

thanks,

greg k-h

On 2026-04-29 13:31:46+02:00, Greg Kroah-Hartman wrote:
> On Wed, Apr 29, 2026 at 01:10:16PM +0200, Linus Probert wrote:
> 
> > Fixes potential speculative cpu oob read in os_intfs.c by guarding the
> > index with array_index_nospec.
> > 
> > Fixes smatch warning:
> > warn: potential spectre issue 'rtw_1d_to_queue' [r]
> 
> Is this value controlled by a user?  Or is it just a normal operation
> that happens that is not controlled?  In other words, can a user
> manipulate this directly to be out of range?
> 
> thanks,
> 
> greg k-h

To my understanding, yes. Which is somewhat limited due to being rather
new to kernel code and not having access to this hardware. The priority
is extracted from ip header which can be user controlled.

However, looking closer at the execution before I see that in both cases
bounding is performed on the value as follows:

	dscp = ip_hdr(skb)->tos & 0xfc;
	prio = dscp >> 5;

So my change here adds no additional security. The smatch warning is a
false positive. It only warned on one of the cases. Most likely because
the bounding happened in a function call and it only sees the u32.

Some quick LLM research told me this (in my own words but have not
verified extensively):

The case where the bounding is performed in a function call could be
susceptible to *Spectre v4 (Speculative Store Bypass)*.
But the fix I applied here only applies to v1 so no additional security
on that front either.

This is probably best to NAK unless we just want to remove a false
positive smatch warning. But I personally don't agree with that.

Br,
Linus

On Wed, Apr 29, 2026 at 02:45:20PM +0200, Linus Probert wrote:
> On 2026-04-29 13:31:46+02:00, Greg Kroah-Hartman wrote:
> > On Wed, Apr 29, 2026 at 01:10:16PM +0200, Linus Probert wrote:
> > 
> > > Fixes potential speculative cpu oob read in os_intfs.c by guarding the
> > > index with array_index_nospec.
> > > 
> > > Fixes smatch warning:
> > > warn: potential spectre issue 'rtw_1d_to_queue' [r]
> > 
> > Is this value controlled by a user?  Or is it just a normal operation
> > that happens that is not controlled?  In other words, can a user
> > manipulate this directly to be out of range?
> > 
> > thanks,
> > 
> > greg k-h
> 
> To my understanding, yes. Which is somewhat limited due to being rather
> new to kernel code and not having access to this hardware. The priority
> is extracted from ip header which can be user controlled.
> 
> However, looking closer at the execution before I see that in both cases
> bounding is performed on the value as follows:
> 
> 	dscp = ip_hdr(skb)->tos & 0xfc;
> 	prio = dscp >> 5;
> 
> So my change here adds no additional security. The smatch warning is a
> false positive. It only warned on one of the cases. Most likely because
> the bounding happened in a function call and it only sees the u32.
> 
> Some quick LLM research told me this (in my own words but have not
> verified extensively):
> 
> The case where the bounding is performed in a function call could be
> susceptible to *Spectre v4 (Speculative Store Bypass)*.
> But the fix I applied here only applies to v1 so no additional security
> on that front either.
> 
> This is probably best to NAK unless we just want to remove a false
> positive smatch warning. But I personally don't agree with that.

Yes, let's fix the tool instead.  The '&' above shows that this is not
really a spectre issue that you can actually trigger.

thanks,

greg k-h