Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751485AbdFHGbI (ORCPT ); Thu, 8 Jun 2017 02:31:08 -0400 Received: from mga01.intel.com ([192.55.52.88]:29441 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750822AbdFHGbG (ORCPT ); Thu, 8 Jun 2017 02:31:06 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.39,313,1493708400"; d="scan'208";a="1179784249" From: "Grumbach, Emmanuel" To: Seraphime Kirkovski , "luca@coelho.fi" CC: "Berg, Johannes" , "Coelho, Luciano" , linuxwifi , Kalle Valo , "open list:INTEL WIRELESS WIFI LINK (iwlwifi)" , "open list:NETWORKING DRIVERS" , open list Subject: RE: [linuxwifi] [PATCH] net: wireless: intel: iwlwifi: dvm: fix tid mask Thread-Topic: [linuxwifi] [PATCH] net: wireless: intel: iwlwifi: dvm: fix tid mask Thread-Index: AQHS395M1Z1I2mCv/kipll1NAceYiKIagERQ Date: Thu, 8 Jun 2017 06:31:01 +0000 Message-ID: <0BA3FCBA62E2DC44AF3030971E174FB3E5520A41@hasmsx107.ger.corp.intel.com> References: <20170607223354.22399-1-kirkseraph@gmail.com> In-Reply-To: <20170607223354.22399-1-kirkseraph@gmail.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 10.0.102.7 dlp-reaction: no-action x-originating-ip: [10.12.126.117] Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id v586VD8N012749 Content-Length: 3500 Lines: 86 Hi, > Subject: [linuxwifi] [PATCH] net: wireless: intel: iwlwifi: dvm: fix tid mask > > Currently the tid mask covers the first 4 bits of iwlagn_tx_resp::ra_tid, > which gives 16 possible values for tid. > This is problematic because IWL_MAX_TID_COUNT is 8, so indexing > iwl_priv::tid_data can go very wrong. True, OTOH we need tid to be 8 sometimes. We *just* need to make sure that we don't index tid_data with this. Hence I think the proper fix is: diff --git a/drivers/net/wireless/intel/iwlwifi/dvm/tx.c b/drivers/net/wireless/intel/iwlwifi/dvm/tx.c index 06ac3f1..16a8646 100644 --- a/drivers/net/wireless/intel/iwlwifi/dvm/tx.c +++ b/drivers/net/wireless/intel/iwlwifi/dvm/tx.c @@ -1190,11 +1190,11 @@ void iwlagn_rx_reply_tx(struct iwl_priv *priv, struct iwl_rx_cmd_buffer *rxb) next_reclaimed; IWL_DEBUG_TX_REPLY(priv, "Next reclaimed packet:%d\n", next_reclaimed); + iwlagn_check_ratid_empty(priv, sta_id, tid); } iwl_trans_reclaim(priv->trans, txq_id, ssn, &skbs); - iwlagn_check_ratid_empty(priv, sta_id, tid); freed = 0; /* process frames */ Clearly calling iwlagn_check_ratid_empty with tid = IWL_TID_NON_QOS is a bad idea. > > With UBSAN I can it happening while establishing the first connection > after module load. > > [ 272.143440] UBSAN: Undefined behaviour in > drivers/net/wireless/intel/iwlwifi/dvm/tx.c:777:32 > [ 272.143447] index 8 is out of range for type 'iwl_tid_data [8]' > [ 272.143457] CPU: 0 PID: 4605 Comm: irq/32-iwlwifi Not tainted 4.12.0-dirty > #2 > [ 272.143460] Hardware name: Hewlett-Packard HP EliteBook 2560p/162B, > BIOS 68SSU Ver. F.02 07/26/2011 > [ 272.143462] Call Trace: > [ 272.143472] dump_stack+0x9c/0x10b > [ 272.143477] ? _atomic_dec_and_lock+0x285/0x285 > [ 272.143486] ubsan_epilogue+0xd/0x4e > [ 272.143493] __ubsan_handle_out_of_bounds+0xef/0x118 > [ 272.143498] ? __ubsan_handle_shift_out_of_bounds+0x221/0x221 > [ 272.143519] ? iwl_trans_pcie_reclaim+0x153/0xc90 [iwlwifi] > [ 272.143539] iwlagn_check_ratid_empty+0x337/0x410 [iwldvm] > [ 272.143556] ? iwl_hcmd_names_cmp+0x2f/0x60 [iwlwifi] > [ 272.143571] iwlagn_rx_reply_tx+0x8a4/0x1820 [iwldvm] > > Signed-off-by: Seraphime Kirkovski > --- > I'm currently running this patch on my machines and I have wifi. > The patch presumes а cleanup patch, I sent yesterday: > https://www.spinics.net/lists/kernel/msg2526314.html > > drivers/net/wireless/intel/iwlwifi/dvm/commands.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/wireless/intel/iwlwifi/dvm/commands.h > b/drivers/net/wireless/intel/iwlwifi/dvm/commands.h > index 37d2ba5ae852..e5994df9ea4c 100644 > --- a/drivers/net/wireless/intel/iwlwifi/dvm/commands.h > +++ b/drivers/net/wireless/intel/iwlwifi/dvm/commands.h > @@ -1448,7 +1448,7 @@ struct agg_tx_status { > */ > /* refer to ra_tid */ > #define IWLAGN_TX_RES_TID_POS 0 > -#define IWLAGN_TX_RES_TID_MSK 0x0f > +#define IWLAGN_TX_RES_TID_MSK 0x07 > #define IWLAGN_TX_RES_RA_POS 4 > #define IWLAGN_TX_RES_RA_MSK 0xf0 > > -- > 2.11.0 > > ------------------------------------- > linuxwifi@eclists.intel.com > https://eclists.intel.com/sympa/info/linuxwifi > Unsubscribe by sending email to sympa@eclists.intel.com with subject > "Unsubscribe linuxwifi"