Use array_index_nospec() to sanitize ridx with respect to speculation.
Signed-off-by: Masashi Honma <[email protected]>
---
net/wireless/nl80211.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 0827cbd..3c469c1 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -3748,6 +3748,7 @@ static bool ht_rateset_to_mask(struct ieee80211_supported_band *sband,
return false;
/* check availability */
+ ridx = array_index_nospec(ridx, IEEE80211_HT_MCS_MASK_LEN);
if (sband->ht_cap.mcs.rx_mask[ridx] & rbit)
mcs[ridx] |= rbit;
else
--
2.7.4
Use array_index_nospec() to sanitize i with respect to speculation.
Signed-off-by: Masashi Honma <[email protected]>
---
net/wireless/nl80211.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 3c469c1..4f47502 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -10227,7 +10227,7 @@ static int cfg80211_cqm_rssi_update(struct cfg80211_registered_device *rdev,
struct wireless_dev *wdev = dev->ieee80211_ptr;
s32 last, low, high;
u32 hyst;
- int i, n;
+ int i, n, low_index;
int err;
/* RSSI reporting disabled? */
@@ -10264,10 +10264,19 @@ static int cfg80211_cqm_rssi_update(struct cfg80211_registered_device *rdev,
if (last < wdev->cqm_config->rssi_thresholds[i])
break;
- low = i > 0 ?
- (wdev->cqm_config->rssi_thresholds[i - 1] - hyst) : S32_MIN;
- high = i < n ?
- (wdev->cqm_config->rssi_thresholds[i] + hyst - 1) : S32_MAX;
+ low_index = i - 1;
+ if (low_index >= 0) {
+ low_index = array_index_nospec(low_index, n);
+ low = wdev->cqm_config->rssi_thresholds[low_index] - hyst;
+ } else {
+ low = S32_MIN;
+ }
+ if (i < n) {
+ i = array_index_nospec(i, n);
+ high = wdev->cqm_config->rssi_thresholds[i] + hyst - 1;
+ } else {
+ high = S32_MAX;
+ }
return rdev_set_cqm_rssi_range_config(rdev, dev, low, high);
}
--
2.7.4
On Tue, 2018-09-25 at 11:15 +0900, Masashi Honma wrote:
> Use array_index_nospec() to sanitize i with respect to speculation.
I applied the first patch in the seies, but I don't understand why this
patch should be necessary.
The value of i isn't controlled by the user, so it shouldn't need to be
sanitized?
The context was *just* missing, added by me:
for (i = 0; i < n; i++)
> if (last < wdev->cqm_config->rssi_thresholds[i])
> break;
This loop determines i, and the user doesn't even control "last", but
even if they did, the possible values of i could only end up being in
the range 0..n-1, so no problems?
johannes
On 2018/09/26 18:23, Johannes Berg wrote:> I applied the first patch in
the seies, but I don't understand why this
> patch should be necessary.
>
> The value of i isn't controlled by the user, so it shouldn't need to be
> sanitized?
>
> The context was *just* missing, added by me:
>
> for (i = 0; i < n; i++)
>> if (last < wdev->cqm_config->rssi_thresholds[i])
>> break;
>
> This loop determines i, and the user doesn't even control "last", but
> even if they did, the possible values of i could only end up being in
> the range 0..n-1, so no problems?
The variable i could be n after the loop when this condition is not
satisfied for all rssi_thresholds[i].
>> if (last < wdev->cqm_config->rssi_thresholds[i])
>> break;
And user could control rssi_thresholds[i] by using
NL80211_ATTR_CQM_RSSI_THOLD.
For example, I could set 4 rssi_thresholds -400, -300, -200, -100.
And then last is -34. I could get i = n = 4 after the loop.
Regards,
Masashi Honma.
On Thu, 2018-09-27 at 07:26 +0900, Masashi Honma wrote:
> On 2018/09/26 18:23, Johannes Berg wrote:> I applied the first patch in
> the seies, but I don't understand why this
> > patch should be necessary.
> >
> > The value of i isn't controlled by the user, so it shouldn't need to be
> > sanitized?
> >
> > The context was *just* missing, added by me:
> >
> > for (i = 0; i < n; i++)
> > > if (last < wdev->cqm_config->rssi_thresholds[i])
> > > break;
> >
> > This loop determines i, and the user doesn't even control "last", but
> > even if they did, the possible values of i could only end up being in
> > the range 0..n-1, so no problems?
>
> The variable i could be n after the loop when this condition is not
> satisfied for all rssi_thresholds[i].
>
> >> if (last < wdev->cqm_config->rssi_thresholds[i])
> >> break;
>
> And user could control rssi_thresholds[i] by using
> NL80211_ATTR_CQM_RSSI_THOLD.
>
> For example, I could set 4 rssi_thresholds -400, -300, -200, -100.
> And then last is -34. I could get i = n = 4 after the loop.
Yes, good point, thanks for the explanation.
I'll merge this then.
johannes