The driver reads a value from hfa384x_from_bap(), which may fail,
and then assigns the value to a local variable. gcc detects that
in in the failure case, the 'rlen' variable now contains
uninitialized data:
In file included from ../drivers/net/wireless/intersil/hostap/hostap_pci.c:220:0:
drivers/net/wireless/intersil/hostap/hostap_hw.c: In function 'hfa384x_get_rid':
drivers/net/wireless/intersil/hostap/hostap_hw.c:842:5: warning: 'rec' may be used uninitialized in this function [-Wmaybe-uninitialized]
if (le16_to_cpu(rec.len) == 0) {
To ensure we get consistent error handling here, this changes the code
to only set rlen if we actually read data correctly, which also takes
care of the warning.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/wireless/intersil/hostap/hostap_hw.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/intersil/hostap/hostap_hw.c b/drivers/net/wireless/intersil/hostap/hostap_hw.c
index 6df3ee561d52..6dbf8ee9490a 100644
--- a/drivers/net/wireless/intersil/hostap/hostap_hw.c
+++ b/drivers/net/wireless/intersil/hostap/hostap_hw.c
@@ -839,12 +839,15 @@ static int hfa384x_get_rid(struct net_device *dev, u16 rid, void *buf, int len,
if (!res)
res = hfa384x_from_bap(dev, BAP0, &rec, sizeof(rec));
- if (le16_to_cpu(rec.len) == 0) {
- /* RID not available */
- res = -ENODATA;
+ if (!res) {
+ if (le16_to_cpu(rec.len) == 0) {
+ /* RID not available */
+ res = -ENODATA;
+ }
+
+ rlen = (le16_to_cpu(rec.len) - 1) * 2;
}
- rlen = (le16_to_cpu(rec.len) - 1) * 2;
if (!res && exact_len && rlen != len) {
printk(KERN_DEBUG "%s: hfa384x_get_rid - RID len mismatch: "
"rid=0x%04x, len=%d (expected %d)\n",--
2.7.0
On Wed, Jan 27, 2016 at 02:45:26PM +0100, Arnd Bergmann wrote:
To ensure we get consistent error handling here, this changes the code
to only set rlen if we actually read data correctly, which also takes
care of the warning.
It may be a good idea to do the job better. Looking at the code:
struct hfa384x_rid_hdr rec;
spin_lock_bh(&local->baplock);
res = hfa384x_setup_bap(dev, BAP0, rid, 0);
if (!res)
res = hfa384x_from_bap(dev, BAP0, &rec, sizeof(rec));
The only thing which initialises any of "rec" is that function call.
The following lines are:
if (le16_to_cpu(rec.len) == 0) {
/* RID not available */
res = -ENODATA;
}
rlen = (le16_to_cpu(rec.len) - 1) * 2;
So, why give the compiler a hard time as you're doing, why make the code
harder to read. What's wrong with:
spin_lock_bh(&local->baplock);
res = hfa384x_setup_bap(dev, BAP0, rid, 0);
if (res)
goto unlock;
res = hfa384x_from_bap(dev, BAP0, &rec, sizeof(rec));
if (res)
goto unlock;
if (le16_to_cpu(rec.len) == 0) {
/* RID not available */
res = -ENODATA;
goto unlock;
}
rlen = (le16_to_cpu(rec.len) - 1) * 2;
if (exact_len && rlen != len) {
printk(KERN_DEBUG "%s: hfa384x_get_rid - RID len mismatch: rid=0x%04x, len=%d (expected %d)\n",
dev->name, rid, rlen, len);
res = -ENODATA;
goto unlock;
}
res = hfa384x_from_bap(dev, BAP0, buf, len);
unlock:
spin_unlock_bh(&local->baplock);
?
--
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
On Wednesday 27 January 2016 19:26:13 Russell King - ARM Linux wrote:
On Wed, Jan 27, 2016 at 02:45:26PM +0100, Arnd Bergmann wrote:
So, why give the compiler a hard time as you're doing, why make the code
harder to read. What's wrong with:
spin_lock_bh(&local->baplock);
res = hfa384x_setup_bap(dev, BAP0, rid, 0);
if (res)
goto unlock;
res = hfa384x_from_bap(dev, BAP0, &rec, sizeof(rec));
if (res)
goto unlock;
if (le16_to_cpu(rec.len) == 0) {
/* RID not available */
res = -ENODATA;
goto unlock;
}
rlen = (le16_to_cpu(rec.len) - 1) * 2;
if (exact_len && rlen != len) {
printk(KERN_DEBUG "%s: hfa384x_get_rid - RID len mismatch: rid=0x%04x, len=%d (expected %d)\n",
dev->name, rid, rlen, len);
res = -ENODATA;
goto unlock;
}
res = hfa384x_from_bap(dev, BAP0, buf, len);
unlock:
spin_unlock_bh(&local->baplock);
?
Good idea, I'll send a new version.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html