b43legacy: fix a lower bounds test
Subsystems (3)
PATCH 1/2 b43legacy: fix a lower bounds test
The problem is that "channel" is an unsigned int, when it's less 5 the value of "channel - 5" is not a negative number as one would expect but is very high positive value instead.
This means that "start" becomes a very high positive value. The result of that is that we never enter the "for (i = start; i <= end; i++) {" loop. Instead of storing the result from b43legacy_radio_aci_detect() it just uses zero.
---
This fix is correct, but making dead code go live can sometimes expose bugs which were previously hiding and is always carries a slight risk.
drivers/net/wireless/broadcom/b43legacy/radio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/wireless/broadcom/b43legacy/radio.c +1 -1
@@ -283,7 +283,7 @@ u8 b43legacy_radio_aci_scan(struct b43legacy_wldev *dev) & 0x7FFF);b43legacy_set_all_gains (dev,3 ,8 ,1 ); - start = (channel -5 >0 ) ? channel -5 :1 ; + start = (channel >5 ) ? channel -5 :1 ; end = (channel +5 <14 ) ? channel +5 :13 ;for (i = start; i <= end; i++) {
Signature · 1 line
2.20.1
PATCH 2/2 b43: fix a lower bounds test
The problem is that "channel" is an unsigned int, when it's less 5 the value of "channel - 5" is not a negative number as one would expect but is very high positive value instead.
This means that "start" becomes a very high positive value. The result of that is that we never enter the "for (i = start; i <= end; i++) {" loop. Instead of storing the result from b43legacy_radio_aci_detect() it just uses zero.
--- drivers/net/wireless/broadcom/b43/phy_g.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/wireless/broadcom/b43/phy_g.c +1 -1
@@ -2297,7 +2297,7 @@ static u8 b43_gphy_aci_scan(struct b43_wldev *dev)b43_phy_mask (dev,B43_PHY_G_CRS ,0x7FFF );b43_set_all_gains (dev,3 ,8 ,1 ); - start = (channel -5 >0 ) ? channel -5 :1 ; + start = (channel >5 ) ? channel -5 :1 ; end = (channel +5 <14 ) ? channel +5 :13 ;for (i = start; i <= end; i++) {
Signature · 7 lines
2.20.1 _______________________________________________ b43-dev mailing list [email protected] http://lists.infradead.org/mailman/listinfo/b43-dev
Re: [PATCH 2/2] b43: fix a lower bounds test
Quoted 20 lines · On Wed, 6 Oct 2021 10:36:22 +0300 Dan Carpenter <[email protected]>…
The problem is that "channel" is an unsigned int, when it's less 5 the value of "channel - 5" is not a negative number as one would expect but is very high positive value instead.
This means that "start" becomes a very high positive value. The result of that is that we never enter the "for (i = start; i <= end; i++) {" loop. Instead of storing the result from
b43legacy_radio_aci_detect()it just uses zero.Fixes ef1a628d83fc ("b43: Implement dynamic PHY API")Signed-off-by Dan Carpenter <[email protected]>--- drivers/net/wireless/broadcom/b43/phy_g.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/net/wireless/broadcom/b43/phy_g.c +0
-0
drivers/net/wireless/broadcom/b43/phy_g.c static u8 b43_gphy_aci_scan(struct b43_wldev *dev) · line 2303
Quoted 8 lines
drivers/net/wireless/broadcom/b43/phy_g.c +1
-1@@ -2297,7 +2297,7 @@ static u8 b43_gphy_aci_scan(struct b43_wldev *dev)b43_phy_mask (dev,B43_PHY_G_CRS ,0x7FFF );b43_set_all_gains (dev,3 ,8 ,1 ); - start = (channel -5 >0 ) ? channel -5 :1 ; + start = (channel >5 ) ? channel -5 :1 ; end = (channel +5 <14 ) ? channel +5 :13 ;for (i = start; i <= end; i++) {
Nice finding.
Signature · 3 lines
Michael https://bues.ch/
Quoted 23 lines · On Wed, 6 Oct 2021 10:35:42 +0300 Dan Carpenter <[email protected]>…
The problem is that "channel" is an unsigned int, when it's less 5 the value of "channel - 5" is not a negative number as one would expect but is very high positive value instead.
This means that "start" becomes a very high positive value. The result of that is that we never enter the "for (i = start; i <= end; i++) {" loop. Instead of storing the result from
b43legacy_radio_aci_detect()it just uses zero.Fixes 75388acd0cd8 ("[B43LEGACY]: add mac80211-based driver for legacy BCM43xx devices")Signed-off-by Dan Carpenter <[email protected]>---
This fix is correct, but making dead code go live can sometimes expose bugs which were previously hiding and is always carries a slight risk.drivers/net/wireless/broadcom/b43legacy/radio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/net/wireless/broadcom/b43legacy/radio.c +0
-0
drivers/net/wireless/broadcom/b43legacy/radio.c u8 b43legacy_radio_aci_scan(struct b43legacy_wldev *dev) · line 289
Quoted 8 lines
drivers/net/wireless/broadcom/b43legacy/radio.c +1
-1@@ -283,7 +283,7 @@ u8 b43legacy_radio_aci_scan(struct b43legacy_wldev *dev) & 0x7FFF);b43legacy_set_all_gains (dev,3 ,8 ,1 ); - start = (channel -5 >0 ) ? channel -5 :1 ; + start = (channel >5 ) ? channel -5 :1 ; end = (channel +5 <14 ) ? channel +5 :13 ;for (i = start; i <= end; i++) {
Nice finding.
Signature · 3 lines
Michael https://bues.ch/
Quoted 4 lines · Dan Carpenter
The problem is that "channel" is an unsigned int, when it's less 5 the value of "channel - 5" is not a negative number as one would expect but is very high positive value instead.
Quoted 8 lines
This means that "start" becomes a very high positive value. The result of that is that we never enter the "for (i = start; i <= end; i++) {" loop. Instead of storing the result from
b43legacy_radio_aci_detect()it just uses zero.Fixes 75388acd0cd8 ("[B43LEGACY]: add mac80211-based driver for legacy BCM43xx devices")Signed-off-by Dan Carpenter <[email protected]>Acked-by Michael Büsch <[email protected]>
2 patches applied to wireless-drivers-next.git, thanks.
c1c8380b0320 b43legacy: fix a lower bounds test9b793db5fca4 b43: fix a lower bounds test
Signature · 3 lines
https://patchwork.kernel.org/project/linux-wireless/patch/20211006073542.GD8404@kili/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches