Received-SPF: pass (google.com: domain of linux-wireless+bounces-3489-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <c9b0c01e-acac-9f15-730f-a0ba991a68dc@candelatech.com>
Date: Mon, 12 Feb 2024 15:22:59 -0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.15.1
Content-Language: en-US
To: linux-wireless <linux-wireless@vger.kernel.org>
From: Ben Greear <greearb@candelatech.com>
Subject: iwlwifi RFC related to iwl_mvm_tx_reclaim
Organization: Candela Technologies
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

Hello,

I'm seeing a crash due to accessing an 'sta' object in this method that is '-2' as integer.

It fails the initial check for null STA, but I'm thinking it might should check for IS_ERR(sta)
as well.

(I have my own patch that references sta before the IS_ERR check later in the code, and this
causes the crash I'm seeing.  I guess upstream will not crash in this situation.).

My question:  Is the patch below a preferred approach, or should I add special checks to where I
access sta and only exit the method lower where it already has the IS_ERR(sta) check?

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/tx.c b/drivers/net/wireless/intel/iwlwifi/mvm/tx.c
index 0567f4eefebc..bd3d2fe424cd 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/tx.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/tx.c
@@ -2337,7 +2337,7 @@ static void iwl_mvm_tx_reclaim(struct iwl_mvm *mvm, int sta_id, int tid,
         sta = rcu_dereference(mvm->fw_id_to_mac_id[sta_id]);

         /* Reclaiming frames for a station that has been deleted ? */
-       if (WARN_ON_ONCE(!sta)) {
+       if (IS_ERR(sta) || !sta) {
                 rcu_read_unlock();
                 return;
         }

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com