qtnfmac: avoid uninitialized variable access

PATCH In review Arnd Bergmann 3 participants 4 messages Oct 9, 2018 – Oct 11, 2018

Subsystems (3)

Networking Network drivers Quantenna qtnfmac wireless driver

lore

Share & tools

lore

Arnd Bergmann

author

2018-10-09 15:57 UTC #0 lore raw

PATCH 1/1 qtnfmac: avoid uninitialized variable access

When qtnf_trans_send_cmd_with_resp() fails, we have not yet initialized 'resp', as pointed out by a valid gcc warning:

drivers/net/wireless/quantenna/qtnfmac/commands.c: In function 'qtnf_cmd_send_with_reply':
drivers/net/wireless/quantenna/qtnfmac/commands.c:133:54: error: 'resp' may be used uninitialized in this function [-Werror=maybe-uninitialized]

Since 'resp_skb' is also not set here, we can skip all further processing and just print the warning and return the failure code.

Fixes c6ed298ffe09 ("qtnfmac: cleanup and unify command error handling")

Signed-off-by Arnd Bergmann <[email protected]>

---
 drivers/net/wireless/quantenna/qtnfmac/commands.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/net/wireless/quantenna/qtnfmac/commands.c +2 -1

index bfdc1ad30c13..9b211459684a 100644
@@ -111,7 +111,7 @@ static int qtnf_cmd_send_with_reply(struct qtnf_bus *bus,
 
 	ret = qtnf_trans_send_cmd_with_resp(bus, cmd_skb, &resp_skb);
 	if (ret)
-		goto out;
+		goto out_noresp;
 
 	resp = (const struct qlink_resp *)resp_skb->data;
 	ret = qtnf_cmd_check_reply_header(resp, cmd_id, mac_id, vif_id,

@@ -132,6 +132,7 @@ static int qtnf_cmd_send_with_reply(struct qtnf_bus *bus,
 	if (!ret && resp)
 		return qtnf_cmd_resp_result_decode(le16_to_cpu(resp->result));
 
+out_noresp:
 	pr_warn("VIF%u.%u: cmd 0x%.4X failed: %d\n",
 		mac_id, vif_id, le16_to_cpu(cmd->cmd_id), ret);

Signature · 1 line

2.18.0

Sergey Matyukevich

Reviewed-by on v1 1/1

2018-10-09 20:25 UTC #1 lore raw

Hello Arnd,

Quoted 11 lines

When qtnf_trans_send_cmd_with_resp() fails, we have not yet initialized 'resp', as pointed out by a valid gcc warning:
drivers/net/wireless/quantenna/qtnfmac/commands.c: In function 'qtnf_cmd_send_with_reply':
drivers/net/wireless/quantenna/qtnfmac/commands.c:133:54: error: 'resp' may be used uninitialized in this function [-Werror=maybe-uninitialized]
Since 'resp_skb' is also not set here, we can skip all further processing and just print the warning and return the failure code.
Fixes c6ed298ffe09 ("qtnfmac: cleanup and unify command error handling")
Signed-off-by Arnd Bergmann <[email protected]>

Thanks for the patch! And for reminding me that I forgot to enable gcc warnings in CI builds in addition to sparse checks.

Reviewed-by Sergey Matyukevich <[email protected]>

Regards,
Sergey

Sergey Matyukevich

2018-10-10 14:56 UTC #2 lore raw

Quoted 9 lines

Quoted 9 lines
When qtnf_trans_send_cmd_with_resp() fails, we have not yet initialized 'resp', as pointed out by a valid gcc warning:
drivers/net/wireless/quantenna/qtnfmac/commands.c: In function 'qtnf_cmd_send_with_reply':
drivers/net/wireless/quantenna/qtnfmac/commands.c:133:54: error: 'resp' may be used uninitialized in this function [-Werror=maybe-uninitialized]
Since 'resp_skb' is also not set here, we can skip all further processing and just print the warning and return the failure code.

Quoted 7 lines

Quoted 2 lines
Fixes c6ed298ffe09 ("qtnfmac: cleanup and unify command error handling")
Signed-off-by Arnd Bergmann <[email protected]>
Thanks for the patch! And for reminding me that I forgot to enable gcc warnings in CI builds in addition to sparse checks.
Reviewed-by Sergey Matyukevich <[email protected]>

Hi Kalle,

Could you please hold back applying this patch for now. We have got another report for the same function, this time static analysis tool warning. It looks like the patch from Arnd does not cover both cases. So we will take a closer look and send a combined fix later.

Regards,
Sergey

Kalle Valo

2018-10-11 08:08 UTC #3 lore raw

Quoted 18 lines · Sergey Matyukevich

Quoted 18 lines
Quoted 9 lines
When qtnf_trans_send_cmd_with_resp() fails, we have not yet initialized 'resp', as pointed out by a valid gcc warning:
drivers/net/wireless/quantenna/qtnfmac/commands.c: In function 'qtnf_cmd_send_with_reply':
drivers/net/wireless/quantenna/qtnfmac/commands.c:133:54: error:
'resp' may be used uninitialized in this function [-Werror=maybe-uninitialized]
Quoted 5 lines
Since 'resp_skb' is also not set here, we can skip all further processing and just print the warning and return the failure code.
Fixes c6ed298ffe09 ("qtnfmac: cleanup and unify command error handling")
Signed-off-by Arnd Bergmann <[email protected]>
Thanks for the patch! And for reminding me that I forgot to enable gcc warnings in CI builds in addition to sparse checks.

Quoted 8 lines

Quoted 1 line
Reviewed-by Sergey Matyukevich <[email protected]>
Hi Kalle,
Could you please hold back applying this patch for now. We have got another report for the same function, this time static analysis tool warning. It looks like the patch from Arnd does not cover both cases. So we will take a closer look and send a combined fix later.

Ok, I'll drop this from my queue. Please resend whatever patch I need to apply.

Signature · 1 line

Kalle Valo