On 6/14/2023 9:58 AM, Dmitry Antipov wrote:
Handle possible 'wait_for_completion_timeout()' errors in 'brcmf_p2p_af_searching_channel()', 'brcmf_p2p_tx_action_frame()' and 'brcmf_p2p_del_vif()', adjust related code. Found by Linux Verification Center (linuxtesting.org) with SVACE.
Thanks for adding this exception handling. Please consider suggestions below.
Reviewed-by: Arend van Spriel <arend.vanspriel@xxxxxxxxxxxx>
Signed-off-by: Dmitry Antipov <dmantipov@xxxxxxxxx> --- v2: rebase against wireless-next tree --- .../broadcom/brcm80211/brcmfmac/p2p.c | 31 +++++++++++++------ 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c index d4492d02e4ea..e43dabdaeb0b 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c @@ -1151,6 +1151,7 @@ static s32 brcmf_p2p_af_searching_channel(struct brcmf_p2p_info *p2p) { struct afx_hdl *afx_hdl = &p2p->afx_hdl; struct brcmf_cfg80211_vif *pri_vif; + bool timeout = false; s32 retry;brcmf_dbg(TRACE, "Enter\n");@@ -1173,8 +1174,11 @@ static s32 brcmf_p2p_af_searching_channel(struct brcmf_p2p_info *p2p) retry); /* search peer on peer's listen channel */ schedule_work(&afx_hdl->afx_work); - wait_for_completion_timeout(&afx_hdl->act_frm_scan, - P2P_AF_FRM_SCAN_MAX_WAIT); + if (!wait_for_completion_timeout(&afx_hdl->act_frm_scan, + P2P_AF_FRM_SCAN_MAX_WAIT)) { + timeout = true; + break; + }
Instead could do: timeout = !wait_for_completion_timeout(...); if (timeout) break;
if ((afx_hdl->peer_chan != P2P_INVALID_CHANNEL) ||
(!test_bit(BRCMF_P2P_STATUS_FINDING_COMMON_CHANNEL,
&p2p->status)))
@@ -1186,8 +1190,11 @@ static s32 brcmf_p2p_af_searching_channel(struct brcmf_p2p_info *p2p)
/* listen on my listen channel */
afx_hdl->is_listen = true;
schedule_work(&afx_hdl->afx_work);
- wait_for_completion_timeout(&afx_hdl->act_frm_scan,
- P2P_AF_FRM_SCAN_MAX_WAIT);
+ if (!wait_for_completion_timeout
+ (&afx_hdl->act_frm_scan, P2P_AF_FRM_SCAN_MAX_WAIT)) {
+ timeout = true;
+ break;
+ }
dito
} if ((afx_hdl->peer_chan != P2P_INVALID_CHANNEL) || (!test_bit(BRCMF_P2P_STATUS_FINDING_COMMON_CHANNEL, @@ -1209,7 +1216,7 @@ static s32 brcmf_p2p_af_searching_channel(struct brcmf_p2p_info *p2p)clear_bit(BRCMF_P2P_STATUS_FINDING_COMMON_CHANNEL, &p2p->status); - return afx_hdl->peer_chan;+ return timeout ? P2P_INVALID_CHANNEL : afx_hdl->peer_chan; }@@ -1580,14 +1587,18 @@ static s32 brcmf_p2p_tx_action_frame(struct brcmf_p2p_info *p2p,(p2p->wait_for_offchan_complete) ? "off-channel" : "on-channel");- wait_for_completion_timeout(&p2p->send_af_done, P2P_AF_MAX_WAIT_TIME);- + if (!wait_for_completion_timeout(&p2p->send_af_done, + P2P_AF_MAX_WAIT_TIME)) { + err = -ETIMEDOUT; + goto clear; + }
Not really needed as timeout would cause the code to proceed in the else branch below.
if (test_bit(BRCMF_P2P_STATUS_ACTION_TX_COMPLETED, &p2p->status)) { brcmf_dbg(TRACE, "TX action frame operation is success\n"); } else { err = -EIO; brcmf_dbg(TRACE, "TX action frame operation has failed\n"); } +clear: /* clear status bit for action tx */ clear_bit(BRCMF_P2P_STATUS_ACTION_TX_COMPLETED, &p2p->status); clear_bit(BRCMF_P2P_STATUS_ACTION_TX_NOACK, &p2p->status); @@ -2404,10 +2415,10 @@ int brcmf_p2p_del_vif(struct wiphy *wiphy, struct wireless_dev *wdev) brcmf_dbg(INFO, "P2P: GO_NEG_PHASE status cleared\n");if (wait_for_disable)- wait_for_completion_timeout(&cfg->vif_disabled, - BRCMF_P2P_DISABLE_TIMEOUT); + err = (wait_for_completion_timeout(&cfg->vif_disabled, + BRCMF_P2P_DISABLE_TIMEOUT) + ? 0 : -ETIMEDOUT);- err = 0;
For P2P_DEVICE wait_for_disable is false so err would be uninitialized here when removing the line above. Looking at the function wait_for_disable is set to true for non-P2P_DEVICE so the wait can move inside the if statement below.
if (iftype != NL80211_IFTYPE_P2P_DEVICE) {
brcmf_vif_clear_mgmt_ies(vif);
err = brcmf_p2p_release_p2p_if(vif);
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature
