Linux-Wireless Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/8] iwlwifi: fixes intended for 5.4 2019-10-04
@ 2019-10-04 13:14 Luca Coelho
  2019-10-04 13:14 ` [PATCH 1/8] iwlwifi: don't access trans_cfg via cfg Luca Coelho
                   ` (7 more replies)
  0 siblings, 8 replies; 11+ messages in thread
From: Luca Coelho @ 2019-10-04 13:14 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless

From: Luca Coelho <luciano.coelho@intel.com>

This is my first patchset with fixes for v5.4.

The changes are:

* fix for an ACPI table parsing bug;
* a fix for a NULL pointer dereference in the cfg with specific
  devices;
* fix the rb_allocator;
* prevent multiple phy configuration with new devices;
* fix a race-condition in the rx queue;
* prevent a couple of memory leaks.

As usual, I'm pushing this to a pending branch, for kbuild bot.  I
will send you a pull-req later with these, since it goes over our
agreed threshold of 5 patches.

Cheers,
Luca.


Haim Dreyfuss (1):
  iwlwifi: mvm: force single phy init

Johannes Berg (2):
  iwlwifi: pcie: fix indexing in command dump for new HW
  iwlwifi: pcie: fix rb_allocator workqueue allocation

Luca Coelho (2):
  iwlwifi: don't access trans_cfg via cfg
  iwlwifi: fix ACPI table revision checks

Naftali Goldstein (1):
  iwlwifi: mvm: fix race in sync rx queue notification

Navid Emamdoost (2):
  iwlwifi: dbg_ini: fix memory leak in alloc_sgtable
  iwlwifi: pcie: fix memory leaks in iwl_pcie_ctxt_info_gen3_init

 drivers/net/wireless/intel/iwlwifi/fw/acpi.c  | 10 +++---
 drivers/net/wireless/intel/iwlwifi/fw/dbg.c   |  1 +
 drivers/net/wireless/intel/iwlwifi/iwl-io.h   | 12 +++----
 drivers/net/wireless/intel/iwlwifi/mvm/fw.c   | 27 ++++++++++----
 .../net/wireless/intel/iwlwifi/mvm/mac80211.c |  9 ++---
 .../intel/iwlwifi/pcie/ctxt-info-gen3.c       | 36 +++++++++++++------
 .../net/wireless/intel/iwlwifi/pcie/trans.c   | 25 +++++++++----
 7 files changed, 83 insertions(+), 37 deletions(-)

-- 
2.23.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/8] iwlwifi: don't access trans_cfg via cfg
  2019-10-04 13:14 [PATCH 0/8] iwlwifi: fixes intended for 5.4 2019-10-04 Luca Coelho
@ 2019-10-04 13:14 ` Luca Coelho
  2019-10-04 13:14 ` [PATCH 2/8] iwlwifi: fix ACPI table revision checks Luca Coelho
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Luca Coelho @ 2019-10-04 13:14 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless

From: Luca Coelho <luciano.coelho@intel.com>

We copy cfg->trans to trans->trans_cfg at the very beginning, so don't
try to access it via cfg->trans anymore, because the cfg may be unset
in later cases.

Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/iwl-io.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-io.h b/drivers/net/wireless/intel/iwlwifi/iwl-io.h
index f8e4f0f5de0c..f09e368c7040 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-io.h
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-io.h
@@ -112,38 +112,38 @@ int iwl_dump_fh(struct iwl_trans *trans, char **buf);
  */
 static inline u32 iwl_umac_prph(struct iwl_trans *trans, u32 ofs)
 {
-	return ofs + trans->cfg->trans.umac_prph_offset;
+	return ofs + trans->trans_cfg->umac_prph_offset;
 }
 
 static inline u32 iwl_read_umac_prph_no_grab(struct iwl_trans *trans, u32 ofs)
 {
 	return iwl_read_prph_no_grab(trans, ofs +
-				     trans->cfg->trans.umac_prph_offset);
+				     trans->trans_cfg->umac_prph_offset);
 }
 
 static inline u32 iwl_read_umac_prph(struct iwl_trans *trans, u32 ofs)
 {
-	return iwl_read_prph(trans, ofs + trans->cfg->trans.umac_prph_offset);
+	return iwl_read_prph(trans, ofs + trans->trans_cfg->umac_prph_offset);
 }
 
 static inline void iwl_write_umac_prph_no_grab(struct iwl_trans *trans, u32 ofs,
 					       u32 val)
 {
-	iwl_write_prph_no_grab(trans,  ofs + trans->cfg->trans.umac_prph_offset,
+	iwl_write_prph_no_grab(trans,  ofs + trans->trans_cfg->umac_prph_offset,
 			       val);
 }
 
 static inline void iwl_write_umac_prph(struct iwl_trans *trans, u32 ofs,
 				       u32 val)
 {
-	iwl_write_prph(trans,  ofs + trans->cfg->trans.umac_prph_offset, val);
+	iwl_write_prph(trans,  ofs + trans->trans_cfg->umac_prph_offset, val);
 }
 
 static inline int iwl_poll_umac_prph_bit(struct iwl_trans *trans, u32 addr,
 					 u32 bits, u32 mask, int timeout)
 {
 	return iwl_poll_prph_bit(trans, addr +
-				 trans->cfg->trans.umac_prph_offset,
+				 trans->trans_cfg->umac_prph_offset,
 				 bits, mask, timeout);
 }
 
-- 
2.23.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 2/8] iwlwifi: fix ACPI table revision checks
  2019-10-04 13:14 [PATCH 0/8] iwlwifi: fixes intended for 5.4 2019-10-04 Luca Coelho
  2019-10-04 13:14 ` [PATCH 1/8] iwlwifi: don't access trans_cfg via cfg Luca Coelho
@ 2019-10-04 13:14 ` Luca Coelho
  2019-10-04 13:14 ` [PATCH 3/8] iwlwifi: mvm: force single phy init Luca Coelho
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Luca Coelho @ 2019-10-04 13:14 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless

From: Luca Coelho <luciano.coelho@intel.com>

We can't check for the ACPI table revision validity in the same if
where we check if the package was read correctly, because we return
PTR_ERR(pkg) and if the table is not valid but the pointer is, we
would return a valid pointer as an error.  Fix that by moving the
table checks to a separate if and return -EINVAL if it's not valid.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/fw/acpi.c | 10 ++++----
 drivers/net/wireless/intel/iwlwifi/mvm/fw.c  | 24 +++++++++++++++-----
 2 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/fw/acpi.c b/drivers/net/wireless/intel/iwlwifi/fw/acpi.c
index 7573af2d88ce..c2db758b9d54 100644
--- a/drivers/net/wireless/intel/iwlwifi/fw/acpi.c
+++ b/drivers/net/wireless/intel/iwlwifi/fw/acpi.c
@@ -162,12 +162,13 @@ int iwl_acpi_get_mcc(struct device *dev, char *mcc)
 
 	wifi_pkg = iwl_acpi_get_wifi_pkg(dev, data, ACPI_WRDD_WIFI_DATA_SIZE,
 					 &tbl_rev);
-	if (IS_ERR(wifi_pkg) || tbl_rev != 0) {
+	if (IS_ERR(wifi_pkg)) {
 		ret = PTR_ERR(wifi_pkg);
 		goto out_free;
 	}
 
-	if (wifi_pkg->package.elements[1].type != ACPI_TYPE_INTEGER) {
+	if (wifi_pkg->package.elements[1].type != ACPI_TYPE_INTEGER ||
+	    tbl_rev != 0) {
 		ret = -EINVAL;
 		goto out_free;
 	}
@@ -224,12 +225,13 @@ int iwl_acpi_get_eckv(struct device *dev, u32 *extl_clk)
 
 	wifi_pkg = iwl_acpi_get_wifi_pkg(dev, data, ACPI_ECKV_WIFI_DATA_SIZE,
 					 &tbl_rev);
-	if (IS_ERR(wifi_pkg) || tbl_rev != 0) {
+	if (IS_ERR(wifi_pkg)) {
 		ret = PTR_ERR(wifi_pkg);
 		goto out_free;
 	}
 
-	if (wifi_pkg->package.elements[1].type != ACPI_TYPE_INTEGER) {
+	if (wifi_pkg->package.elements[1].type != ACPI_TYPE_INTEGER ||
+	    tbl_rev != 0) {
 		ret = -EINVAL;
 		goto out_free;
 	}
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
index 32a5e4e5461f..b2eb18eedf02 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
@@ -694,12 +694,13 @@ static int iwl_mvm_sar_get_wrds_table(struct iwl_mvm *mvm)
 
 	wifi_pkg = iwl_acpi_get_wifi_pkg(mvm->dev, data,
 					 ACPI_WRDS_WIFI_DATA_SIZE, &tbl_rev);
-	if (IS_ERR(wifi_pkg) || tbl_rev != 0) {
+	if (IS_ERR(wifi_pkg)) {
 		ret = PTR_ERR(wifi_pkg);
 		goto out_free;
 	}
 
-	if (wifi_pkg->package.elements[1].type != ACPI_TYPE_INTEGER) {
+	if (wifi_pkg->package.elements[1].type != ACPI_TYPE_INTEGER ||
+	    tbl_rev != 0) {
 		ret = -EINVAL;
 		goto out_free;
 	}
@@ -731,13 +732,14 @@ static int iwl_mvm_sar_get_ewrd_table(struct iwl_mvm *mvm)
 
 	wifi_pkg = iwl_acpi_get_wifi_pkg(mvm->dev, data,
 					 ACPI_EWRD_WIFI_DATA_SIZE, &tbl_rev);
-	if (IS_ERR(wifi_pkg) || tbl_rev != 0) {
+	if (IS_ERR(wifi_pkg)) {
 		ret = PTR_ERR(wifi_pkg);
 		goto out_free;
 	}
 
 	if ((wifi_pkg->package.elements[1].type != ACPI_TYPE_INTEGER) ||
-	    (wifi_pkg->package.elements[2].type != ACPI_TYPE_INTEGER)) {
+	    (wifi_pkg->package.elements[2].type != ACPI_TYPE_INTEGER) ||
+	    tbl_rev != 0) {
 		ret = -EINVAL;
 		goto out_free;
 	}
@@ -791,11 +793,16 @@ static int iwl_mvm_sar_get_wgds_table(struct iwl_mvm *mvm)
 
 	wifi_pkg = iwl_acpi_get_wifi_pkg(mvm->dev, data,
 					 ACPI_WGDS_WIFI_DATA_SIZE, &tbl_rev);
-	if (IS_ERR(wifi_pkg) || tbl_rev > 1) {
+	if (IS_ERR(wifi_pkg)) {
 		ret = PTR_ERR(wifi_pkg);
 		goto out_free;
 	}
 
+	if (tbl_rev != 0) {
+		ret = -EINVAL;
+		goto out_free;
+	}
+
 	mvm->geo_rev = tbl_rev;
 	for (i = 0; i < ACPI_NUM_GEO_PROFILES; i++) {
 		for (j = 0; j < ACPI_GEO_TABLE_SIZE; j++) {
@@ -1020,11 +1027,16 @@ static int iwl_mvm_get_ppag_table(struct iwl_mvm *mvm)
 	wifi_pkg = iwl_acpi_get_wifi_pkg(mvm->dev, data,
 					 ACPI_PPAG_WIFI_DATA_SIZE, &tbl_rev);
 
-	if (IS_ERR(wifi_pkg) || tbl_rev != 0) {
+	if (IS_ERR(wifi_pkg)) {
 		ret = PTR_ERR(wifi_pkg);
 		goto out_free;
 	}
 
+	if (tbl_rev != 0) {
+		ret = -EINVAL;
+		goto out_free;
+	}
+
 	enabled = &wifi_pkg->package.elements[1];
 	if (enabled->type != ACPI_TYPE_INTEGER ||
 	    (enabled->integer.value != 0 && enabled->integer.value != 1)) {
-- 
2.23.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 3/8] iwlwifi: mvm: force single phy init
  2019-10-04 13:14 [PATCH 0/8] iwlwifi: fixes intended for 5.4 2019-10-04 Luca Coelho
  2019-10-04 13:14 ` [PATCH 1/8] iwlwifi: don't access trans_cfg via cfg Luca Coelho
  2019-10-04 13:14 ` [PATCH 2/8] iwlwifi: fix ACPI table revision checks Luca Coelho
@ 2019-10-04 13:14 ` Luca Coelho
  2019-10-04 13:14 ` [PATCH 4/8] iwlwifi: mvm: fix race in sync rx queue notification Luca Coelho
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Luca Coelho @ 2019-10-04 13:14 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless

From: Haim Dreyfuss <haim.dreyfuss@intel.com>

The PHY is initialized during device initialization, but devices with
the tx_siso_diversity flag set need to send PHY_CONFIGURATION_CMD first,
otherwise the PHY would be reinitialized, causing a SYSASSERT.

To fix this, use a bit that tells the FW not to complete the PHY
initialization before a PHY_CONFIGURATION_CMD is received.

Signed-off-by: Haim Dreyfuss <haim.dreyfuss@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/mvm/fw.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
index b2eb18eedf02..0d2229319261 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
@@ -420,6 +420,9 @@ static int iwl_run_unified_mvm_ucode(struct iwl_mvm *mvm, bool read_nvm)
 	};
 	int ret;
 
+	if (mvm->trans->cfg->tx_with_siso_diversity)
+		init_cfg.init_flags |= cpu_to_le32(BIT(IWL_INIT_PHY));
+
 	lockdep_assert_held(&mvm->mutex);
 
 	mvm->rfkill_safe_init_done = false;
-- 
2.23.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 4/8] iwlwifi: mvm: fix race in sync rx queue notification
  2019-10-04 13:14 [PATCH 0/8] iwlwifi: fixes intended for 5.4 2019-10-04 Luca Coelho
                   ` (2 preceding siblings ...)
  2019-10-04 13:14 ` [PATCH 3/8] iwlwifi: mvm: force single phy init Luca Coelho
@ 2019-10-04 13:14 ` Luca Coelho
  2019-10-04 13:41   ` Kalle Valo
  2019-10-04 13:14 ` [PATCH 5/8] iwlwifi: pcie: fix indexing in command dump for new HW Luca Coelho
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 11+ messages in thread
From: Luca Coelho @ 2019-10-04 13:14 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless

From: Naftali Goldstein <naftali.goldstein@intel.com>

Consider the following flow:
 1. Driver starts to sync the rx queues due to a delba.
    mvm->queue_sync_cookie=1.
    This rx-queues-sync is synchronous, so it doesn't increment the
    cookie until all rx queues handle the notification from FW.
 2. During this time, driver starts to sync rx queues due to nssn sync
    required.
    The cookie's value is still 1, but it doesn't matter since this
    rx-queue-sync is non-synchronous so in the notification handler the
    cookie is ignored.
    What _does_ matter is that this flow increments the cookie to 2
    immediately.
    Remember though that the FW won't start servicing this command until
    it's done with the previous one.
 3. FW is still handling the first command, so it sends a notification
    with internal_notif->sync=1, and internal_notif->cookie=0, which
    triggers a WARN_ONCE.

The solution for this race is to only use the mvm->queue_sync_cookie in
case of a synchronous sync-rx-queues. This way in step 2 the cookie's
value won't change so we avoid the WARN.

The commit in the "fixes" field is the first commit to introduce
non-synchronous sending of this command to FW.

Signed-off-by: Naftali Goldstein <naftali.goldstein@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c b/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
index cd1b10042fbf..d31f96c3f925 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
@@ -4881,11 +4881,11 @@ void iwl_mvm_sync_rx_queues_internal(struct iwl_mvm *mvm,
 	if (!iwl_mvm_has_new_rx_api(mvm))
 		return;
 
-	notif->cookie = mvm->queue_sync_cookie;
-
-	if (notif->sync)
+	if (notif->sync) {
+		notif->cookie = mvm->queue_sync_cookie;
 		atomic_set(&mvm->queue_sync_counter,
 			   mvm->trans->num_rx_queues);
+	}
 
 	ret = iwl_mvm_notify_rx_queue(mvm, qmask, (u8 *)notif,
 				      size, !notif->sync);
@@ -4905,7 +4905,8 @@ void iwl_mvm_sync_rx_queues_internal(struct iwl_mvm *mvm,
 
 out:
 	atomic_set(&mvm->queue_sync_counter, 0);
-	mvm->queue_sync_cookie++;
+	if (notif->sync)
+		mvm->queue_sync_cookie++;
 }
 
 static void iwl_mvm_sync_rx_queues(struct ieee80211_hw *hw)
-- 
2.23.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 5/8] iwlwifi: pcie: fix indexing in command dump for new HW
  2019-10-04 13:14 [PATCH 0/8] iwlwifi: fixes intended for 5.4 2019-10-04 Luca Coelho
                   ` (3 preceding siblings ...)
  2019-10-04 13:14 ` [PATCH 4/8] iwlwifi: mvm: fix race in sync rx queue notification Luca Coelho
@ 2019-10-04 13:14 ` Luca Coelho
  2019-10-04 13:14 ` [PATCH 6/8] iwlwifi: pcie: fix rb_allocator workqueue allocation Luca Coelho
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Luca Coelho @ 2019-10-04 13:14 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless

From: Johannes Berg <johannes.berg@intel.com>

We got a crash in iwl_trans_pcie_get_cmdlen(), while the TFD was
being accessed to sum up the lengths.

We want to access the TFD here, which is the information for the
hardware. We always only allocate 32 buffers for the cmd queue,
but on newer hardware (using TFH) we can also allocate only a
shorter hardware array, also only 32 TFDs. Prior to the TFH, we
had to allocate a bigger TFD array but would make those point to
a smaller set of buffers.

Additionally, now max_tfd_queue_size is up to 65536, so we can
access *way* out of bounds of a really only 32-entry array, so
it crashes.

Fix this by making the TFD index depend on which hardware we are
using right now.

While changing the calculation, also fix it to not use void ptr
arithmetic, but cast to u8 * before.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/pcie/trans.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
index f8a1f985a1d8..ab7480a85015 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
@@ -3272,11 +3272,17 @@ static struct iwl_trans_dump_data
 		ptr = cmdq->write_ptr;
 		for (i = 0; i < cmdq->n_window; i++) {
 			u8 idx = iwl_pcie_get_cmd_index(cmdq, ptr);
+			u8 tfdidx;
 			u32 caplen, cmdlen;
 
+			if (trans->trans_cfg->use_tfh)
+				tfdidx = idx;
+			else
+				tfdidx = ptr;
+
 			cmdlen = iwl_trans_pcie_get_cmdlen(trans,
-							   cmdq->tfds +
-							   tfd_size * ptr);
+							   (u8 *)cmdq->tfds +
+							   tfd_size * tfdidx);
 			caplen = min_t(u32, TFD_MAX_PAYLOAD_SIZE, cmdlen);
 
 			if (cmdlen) {
-- 
2.23.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 6/8] iwlwifi: pcie: fix rb_allocator workqueue allocation
  2019-10-04 13:14 [PATCH 0/8] iwlwifi: fixes intended for 5.4 2019-10-04 Luca Coelho
                   ` (4 preceding siblings ...)
  2019-10-04 13:14 ` [PATCH 5/8] iwlwifi: pcie: fix indexing in command dump for new HW Luca Coelho
@ 2019-10-04 13:14 ` Luca Coelho
  2019-10-04 13:14 ` [PATCH 7/8] iwlwifi: dbg_ini: fix memory leak in alloc_sgtable Luca Coelho
  2019-10-04 13:14 ` [PATCH 8/8] iwlwifi: pcie: fix memory leaks in iwl_pcie_ctxt_info_gen3_init Luca Coelho
  7 siblings, 0 replies; 11+ messages in thread
From: Luca Coelho @ 2019-10-04 13:14 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless

From: Johannes Berg <johannes.berg@intel.com>

We don't handle failures in the rb_allocator workqueue allocation
correctly. To fix that, move the code earlier so the cleanup is
easier and we don't have to undo all the interrupt allocations in
this case.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/pcie/trans.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
index ab7480a85015..6961f00ff812 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
@@ -3456,6 +3456,15 @@ struct iwl_trans *iwl_trans_pcie_alloc(struct pci_dev *pdev,
 	spin_lock_init(&trans_pcie->reg_lock);
 	mutex_init(&trans_pcie->mutex);
 	init_waitqueue_head(&trans_pcie->ucode_write_waitq);
+
+	trans_pcie->rba.alloc_wq = alloc_workqueue("rb_allocator",
+						   WQ_HIGHPRI | WQ_UNBOUND, 1);
+	if (!trans_pcie->rba.alloc_wq) {
+		ret = -ENOMEM;
+		goto out_free_trans;
+	}
+	INIT_WORK(&trans_pcie->rba.rx_alloc, iwl_pcie_rx_allocator_work);
+
 	trans_pcie->tso_hdr_page = alloc_percpu(struct iwl_tso_hdr_page);
 	if (!trans_pcie->tso_hdr_page) {
 		ret = -ENOMEM;
@@ -3590,10 +3599,6 @@ struct iwl_trans *iwl_trans_pcie_alloc(struct pci_dev *pdev,
 		trans_pcie->inta_mask = CSR_INI_SET_MASK;
 	 }
 
-	trans_pcie->rba.alloc_wq = alloc_workqueue("rb_allocator",
-						   WQ_HIGHPRI | WQ_UNBOUND, 1);
-	INIT_WORK(&trans_pcie->rba.rx_alloc, iwl_pcie_rx_allocator_work);
-
 #ifdef CONFIG_IWLWIFI_DEBUGFS
 	trans_pcie->fw_mon_data.state = IWL_FW_MON_DBGFS_STATE_CLOSED;
 	mutex_init(&trans_pcie->fw_mon_data.mutex);
@@ -3605,6 +3610,8 @@ struct iwl_trans *iwl_trans_pcie_alloc(struct pci_dev *pdev,
 	iwl_pcie_free_ict(trans);
 out_no_pci:
 	free_percpu(trans_pcie->tso_hdr_page);
+	destroy_workqueue(trans_pcie->rba.alloc_wq);
+out_free_trans:
 	iwl_trans_free(trans);
 	return ERR_PTR(ret);
 }
-- 
2.23.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 7/8] iwlwifi: dbg_ini: fix memory leak in alloc_sgtable
  2019-10-04 13:14 [PATCH 0/8] iwlwifi: fixes intended for 5.4 2019-10-04 Luca Coelho
                   ` (5 preceding siblings ...)
  2019-10-04 13:14 ` [PATCH 6/8] iwlwifi: pcie: fix rb_allocator workqueue allocation Luca Coelho
@ 2019-10-04 13:14 ` Luca Coelho
  2019-10-04 13:14 ` [PATCH 8/8] iwlwifi: pcie: fix memory leaks in iwl_pcie_ctxt_info_gen3_init Luca Coelho
  7 siblings, 0 replies; 11+ messages in thread
From: Luca Coelho @ 2019-10-04 13:14 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless

From: Navid Emamdoost <navid.emamdoost@gmail.com>

In alloc_sgtable if alloc_page fails, the alocated table should be
released.

Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/fw/dbg.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/intel/iwlwifi/fw/dbg.c b/drivers/net/wireless/intel/iwlwifi/fw/dbg.c
index 5c8602de9168..87421807e040 100644
--- a/drivers/net/wireless/intel/iwlwifi/fw/dbg.c
+++ b/drivers/net/wireless/intel/iwlwifi/fw/dbg.c
@@ -646,6 +646,7 @@ static struct scatterlist *alloc_sgtable(int size)
 				if (new_page)
 					__free_page(new_page);
 			}
+			kfree(table);
 			return NULL;
 		}
 		alloc_size = min_t(int, size, PAGE_SIZE);
-- 
2.23.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 8/8] iwlwifi: pcie: fix memory leaks in iwl_pcie_ctxt_info_gen3_init
  2019-10-04 13:14 [PATCH 0/8] iwlwifi: fixes intended for 5.4 2019-10-04 Luca Coelho
                   ` (6 preceding siblings ...)
  2019-10-04 13:14 ` [PATCH 7/8] iwlwifi: dbg_ini: fix memory leak in alloc_sgtable Luca Coelho
@ 2019-10-04 13:14 ` Luca Coelho
  7 siblings, 0 replies; 11+ messages in thread
From: Luca Coelho @ 2019-10-04 13:14 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless

From: Navid Emamdoost <navid.emamdoost@gmail.com>

In iwl_pcie_ctxt_info_gen3_init there are cases that the allocated dma
memory is leaked in case of error.

DMA memories prph_scratch, prph_info, and ctxt_info_gen3 are allocated
and initialized to be later assigned to trans_pcie. But in any error case
before such assignment the allocated memories should be released.

First of such error cases happens when iwl_pcie_init_fw_sec fails.
Current implementation correctly releases prph_scratch. But in two
sunsequent error cases where dma_alloc_coherent may fail, such
releases are missing.

This commit adds release for prph_scratch when allocation for
prph_info fails, and adds releases for prph_scratch and prph_info when
allocation for ctxt_info_gen3 fails.

Fixes: 2ee824026288 ("iwlwifi: pcie: support context information for 22560 devices")
Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 .../intel/iwlwifi/pcie/ctxt-info-gen3.c       | 36 +++++++++++++------
 1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/ctxt-info-gen3.c b/drivers/net/wireless/intel/iwlwifi/pcie/ctxt-info-gen3.c
index 75fa8a6aafee..74980382e64c 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/ctxt-info-gen3.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/ctxt-info-gen3.c
@@ -107,13 +107,9 @@ int iwl_pcie_ctxt_info_gen3_init(struct iwl_trans *trans,
 
 	/* allocate ucode sections in dram and set addresses */
 	ret = iwl_pcie_init_fw_sec(trans, fw, &prph_scratch->dram);
-	if (ret) {
-		dma_free_coherent(trans->dev,
-				  sizeof(*prph_scratch),
-				  prph_scratch,
-				  trans_pcie->prph_scratch_dma_addr);
-		return ret;
-	}
+	if (ret)
+		goto err_free_prph_scratch;
+
 
 	/* Allocate prph information
 	 * currently we don't assign to the prph info anything, but it would get
@@ -121,16 +117,20 @@ int iwl_pcie_ctxt_info_gen3_init(struct iwl_trans *trans,
 	prph_info = dma_alloc_coherent(trans->dev, sizeof(*prph_info),
 				       &trans_pcie->prph_info_dma_addr,
 				       GFP_KERNEL);
-	if (!prph_info)
-		return -ENOMEM;
+	if (!prph_info) {
+		ret = -ENOMEM;
+		goto err_free_prph_scratch;
+	}
 
 	/* Allocate context info */
 	ctxt_info_gen3 = dma_alloc_coherent(trans->dev,
 					    sizeof(*ctxt_info_gen3),
 					    &trans_pcie->ctxt_info_dma_addr,
 					    GFP_KERNEL);
-	if (!ctxt_info_gen3)
-		return -ENOMEM;
+	if (!ctxt_info_gen3) {
+		ret = -ENOMEM;
+		goto err_free_prph_info;
+	}
 
 	ctxt_info_gen3->prph_info_base_addr =
 		cpu_to_le64(trans_pcie->prph_info_dma_addr);
@@ -186,6 +186,20 @@ int iwl_pcie_ctxt_info_gen3_init(struct iwl_trans *trans,
 		iwl_set_bit(trans, CSR_GP_CNTRL, CSR_AUTO_FUNC_INIT);
 
 	return 0;
+
+err_free_prph_info:
+	dma_free_coherent(trans->dev,
+			  sizeof(*prph_info),
+			prph_info,
+			trans_pcie->prph_info_dma_addr);
+
+err_free_prph_scratch:
+	dma_free_coherent(trans->dev,
+			  sizeof(*prph_scratch),
+			prph_scratch,
+			trans_pcie->prph_scratch_dma_addr);
+	return ret;
+
 }
 
 void iwl_pcie_ctxt_info_gen3_free(struct iwl_trans *trans)
-- 
2.23.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 4/8] iwlwifi: mvm: fix race in sync rx queue notification
  2019-10-04 13:14 ` [PATCH 4/8] iwlwifi: mvm: fix race in sync rx queue notification Luca Coelho
@ 2019-10-04 13:41   ` Kalle Valo
  2019-10-04 18:06     ` Luca Coelho
  0 siblings, 1 reply; 11+ messages in thread
From: Kalle Valo @ 2019-10-04 13:41 UTC (permalink / raw)
  To: Luca Coelho; +Cc: linux-wireless

Luca Coelho <luca@coelho.fi> writes:

> From: Naftali Goldstein <naftali.goldstein@intel.com>
>
> Consider the following flow:
>  1. Driver starts to sync the rx queues due to a delba.
>     mvm->queue_sync_cookie=1.
>     This rx-queues-sync is synchronous, so it doesn't increment the
>     cookie until all rx queues handle the notification from FW.
>  2. During this time, driver starts to sync rx queues due to nssn sync
>     required.
>     The cookie's value is still 1, but it doesn't matter since this
>     rx-queue-sync is non-synchronous so in the notification handler the
>     cookie is ignored.
>     What _does_ matter is that this flow increments the cookie to 2
>     immediately.
>     Remember though that the FW won't start servicing this command until
>     it's done with the previous one.
>  3. FW is still handling the first command, so it sends a notification
>     with internal_notif->sync=1, and internal_notif->cookie=0, which
>     triggers a WARN_ONCE.
>
> The solution for this race is to only use the mvm->queue_sync_cookie in
> case of a synchronous sync-rx-queues. This way in step 2 the cookie's
> value won't change so we avoid the WARN.
>
> The commit in the "fixes" field is the first commit to introduce
> non-synchronous sending of this command to FW.

But I don't see a Fixes field anywhere :)

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 4/8] iwlwifi: mvm: fix race in sync rx queue notification
  2019-10-04 13:41   ` Kalle Valo
@ 2019-10-04 18:06     ` Luca Coelho
  0 siblings, 0 replies; 11+ messages in thread
From: Luca Coelho @ 2019-10-04 18:06 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless

On Fri, 2019-10-04 at 16:41 +0300, Kalle Valo wrote:
> Luca Coelho <luca@coelho.fi> writes:
> 
> > From: Naftali Goldstein <naftali.goldstein@intel.com>
> > 
> > Consider the following flow:
> >  1. Driver starts to sync the rx queues due to a delba.
> >     mvm->queue_sync_cookie=1.
> >     This rx-queues-sync is synchronous, so it doesn't increment the
> >     cookie until all rx queues handle the notification from FW.
> >  2. During this time, driver starts to sync rx queues due to nssn sync
> >     required.
> >     The cookie's value is still 1, but it doesn't matter since this
> >     rx-queue-sync is non-synchronous so in the notification handler the
> >     cookie is ignored.
> >     What _does_ matter is that this flow increments the cookie to 2
> >     immediately.
> >     Remember though that the FW won't start servicing this command until
> >     it's done with the previous one.
> >  3. FW is still handling the first command, so it sends a notification
> >     with internal_notif->sync=1, and internal_notif->cookie=0, which
> >     triggers a WARN_ONCE.
> > 
> > The solution for this race is to only use the mvm->queue_sync_cookie in
> > case of a synchronous sync-rx-queues. This way in step 2 the cookie's
> > value won't change so we avoid the WARN.
> > 
> > The commit in the "fixes" field is the first commit to introduce
> > non-synchronous sending of this command to FW.
> 
> But I don't see a Fixes field anywhere :)

Hmmm, good catch.  My script should have added it.  One more thing to
check... *sigh*

This is the aforementioned commit:

Fixes: 3c514bf831ac ("iwlwifi: mvm: add a loose synchronization of the NSSN across Rx queues")

I'll add it and include it when I send the pull-req.

Thanks!

--
Cheers,
Luca.


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, back to index

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-04 13:14 [PATCH 0/8] iwlwifi: fixes intended for 5.4 2019-10-04 Luca Coelho
2019-10-04 13:14 ` [PATCH 1/8] iwlwifi: don't access trans_cfg via cfg Luca Coelho
2019-10-04 13:14 ` [PATCH 2/8] iwlwifi: fix ACPI table revision checks Luca Coelho
2019-10-04 13:14 ` [PATCH 3/8] iwlwifi: mvm: force single phy init Luca Coelho
2019-10-04 13:14 ` [PATCH 4/8] iwlwifi: mvm: fix race in sync rx queue notification Luca Coelho
2019-10-04 13:41   ` Kalle Valo
2019-10-04 18:06     ` Luca Coelho
2019-10-04 13:14 ` [PATCH 5/8] iwlwifi: pcie: fix indexing in command dump for new HW Luca Coelho
2019-10-04 13:14 ` [PATCH 6/8] iwlwifi: pcie: fix rb_allocator workqueue allocation Luca Coelho
2019-10-04 13:14 ` [PATCH 7/8] iwlwifi: dbg_ini: fix memory leak in alloc_sgtable Luca Coelho
2019-10-04 13:14 ` [PATCH 8/8] iwlwifi: pcie: fix memory leaks in iwl_pcie_ctxt_info_gen3_init Luca Coelho

Linux-Wireless Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-wireless/0 linux-wireless/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-wireless linux-wireless/ https://lore.kernel.org/linux-wireless \
		linux-wireless@vger.kernel.org linux-wireless@archiver.kernel.org
	public-inbox-index linux-wireless

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-wireless


AGPL code for this site: git clone https://public-inbox.org/ public-inbox