[PATCH] iwlwifi: actually check allocated conf_tlv pointer

[PATCH] iwlwifi: actually check allocated conf_tlv pointer

[Chris Rorvick]

Commit 71bc0334a637 ("iwlwifi: check allocated pointer when allocating
conf_tlvs") attempted to fix a typoe introduced by commit 17b809c9b22e
("iwlwifi: dbg: move debug data to a struct") but does not implement the
check correctly.

Tweeted-by: @grsecurity
Signed-off-by: Chris Rorvick <chris@rorvick.com>
---
In this wasn't picked up?

 drivers/net/wireless/intel/iwlwifi/iwl-drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
index ff52e69c1c80..a37f330e7bd4 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
@@ -1465,11 +1465,11 @@ static void iwl_req_fw_callback(const struct firmware *ucode_raw, void *context)
 		if (pieces->dbg_conf_tlv[i]) {
 			drv->fw.dbg.conf_tlv[i] =
 				kmemdup(pieces->dbg_conf_tlv[i],
 					pieces->dbg_conf_tlv_len[i],
 					GFP_KERNEL);
-			if (!pieces->dbg_conf_tlv[i])
+			if (!drv->fw.dbg_conf_tlv[i])
 				goto out_free_fw;
 		}
 	}
 
 	memset(&trigger_tlv_sz, 0xff, sizeof(trigger_tlv_sz));
-- 
2.24.1

Re: [PATCH] iwlwifi: actually check allocated conf_tlv pointer

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 13447 bytes --]

Hi Chris,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on wireless-drivers/master]
[also build test ERROR on v5.6 next-20200402]
[cannot apply to wireless-drivers-next/master]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Chris-Rorvick/iwlwifi-actually-check-allocated-conf_tlv-pointer/20200402-145204
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers.git master
config: mips-ip27_defconfig (attached as .config)
compiler: mips64-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=9.3.0 make.cross ARCH=mips 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/net/wireless/intel/iwlwifi/iwl-drv.c: In function 'iwl_req_fw_callback':
>> drivers/net/wireless/intel/iwlwifi/iwl-drv.c:1470:16: error: 'struct iwl_fw' has no member named 'dbg_conf_tlv'
    1470 |    if (!drv->fw.dbg_conf_tlv[i])
         |                ^

vim +1470 drivers/net/wireless/intel/iwlwifi/iwl-drv.c

  1323	
  1324	/**
  1325	 * iwl_req_fw_callback - callback when firmware was loaded
  1326	 *
  1327	 * If loaded successfully, copies the firmware into buffers
  1328	 * for the card to fetch (via DMA).
  1329	 */
  1330	static void iwl_req_fw_callback(const struct firmware *ucode_raw, void *context)
  1331	{
  1332		struct iwl_drv *drv = context;
  1333		struct iwl_fw *fw = &drv->fw;
  1334		struct iwl_ucode_header *ucode;
  1335		struct iwlwifi_opmode_table *op;
  1336		int err;
  1337		struct iwl_firmware_pieces *pieces;
  1338		const unsigned int api_max = drv->trans->cfg->ucode_api_max;
  1339		const unsigned int api_min = drv->trans->cfg->ucode_api_min;
  1340		size_t trigger_tlv_sz[FW_DBG_TRIGGER_MAX];
  1341		u32 api_ver;
  1342		int i;
  1343		bool load_module = false;
  1344		bool usniffer_images = false;
  1345	
  1346		fw->ucode_capa.max_probe_length = IWL_DEFAULT_MAX_PROBE_LENGTH;
  1347		fw->ucode_capa.standard_phy_calibration_size =
  1348				IWL_DEFAULT_STANDARD_PHY_CALIBRATE_TBL_SIZE;
  1349		fw->ucode_capa.n_scan_channels = IWL_DEFAULT_SCAN_CHANNELS;
  1350		/* dump all fw memory areas by default */
  1351		fw->dbg.dump_mask = 0xffffffff;
  1352	
  1353		pieces = kzalloc(sizeof(*pieces), GFP_KERNEL);
  1354		if (!pieces)
  1355			goto out_free_fw;
  1356	
  1357		if (!ucode_raw)
  1358			goto try_again;
  1359	
  1360		IWL_DEBUG_FW_INFO(drv, "Loaded firmware file '%s' (%zd bytes).\n",
  1361				  drv->firmware_name, ucode_raw->size);
  1362	
  1363		/* Make sure that we got at least the API version number */
  1364		if (ucode_raw->size < 4) {
  1365			IWL_ERR(drv, "File size way too small!\n");
  1366			goto try_again;
  1367		}
  1368	
  1369		/* Data from ucode file:  header followed by uCode images */
  1370		ucode = (struct iwl_ucode_header *)ucode_raw->data;
  1371	
  1372		if (ucode->ver)
  1373			err = iwl_parse_v1_v2_firmware(drv, ucode_raw, pieces);
  1374		else
  1375			err = iwl_parse_tlv_firmware(drv, ucode_raw, pieces,
  1376						     &fw->ucode_capa, &usniffer_images);
  1377	
  1378		if (err)
  1379			goto try_again;
  1380	
  1381		if (fw_has_api(&drv->fw.ucode_capa, IWL_UCODE_TLV_API_NEW_VERSION))
  1382			api_ver = drv->fw.ucode_ver;
  1383		else
  1384			api_ver = IWL_UCODE_API(drv->fw.ucode_ver);
  1385	
  1386		/*
  1387		 * api_ver should match the api version forming part of the
  1388		 * firmware filename ... but we don't check for that and only rely
  1389		 * on the API version read from firmware header from here on forward
  1390		 */
  1391		if (api_ver < api_min || api_ver > api_max) {
  1392			IWL_ERR(drv,
  1393				"Driver unable to support your firmware API. "
  1394				"Driver supports v%u, firmware is v%u.\n",
  1395				api_max, api_ver);
  1396			goto try_again;
  1397		}
  1398	
  1399		/*
  1400		 * In mvm uCode there is no difference between data and instructions
  1401		 * sections.
  1402		 */
  1403		if (fw->type == IWL_FW_DVM && validate_sec_sizes(drv, pieces,
  1404								 drv->trans->cfg))
  1405			goto try_again;
  1406	
  1407		/* Allocate ucode buffers for card's bus-master loading ... */
  1408	
  1409		/* Runtime instructions and 2 copies of data:
  1410		 * 1) unmodified from disk
  1411		 * 2) backup cache for save/restore during power-downs
  1412		 */
  1413		for (i = 0; i < IWL_UCODE_TYPE_MAX; i++)
  1414			if (iwl_alloc_ucode(drv, pieces, i))
  1415				goto out_free_fw;
  1416	
  1417		if (pieces->dbg_dest_tlv_init) {
  1418			size_t dbg_dest_size = sizeof(*drv->fw.dbg.dest_tlv) +
  1419				sizeof(drv->fw.dbg.dest_tlv->reg_ops[0]) *
  1420				drv->fw.dbg.n_dest_reg;
  1421	
  1422			drv->fw.dbg.dest_tlv = kmalloc(dbg_dest_size, GFP_KERNEL);
  1423	
  1424			if (!drv->fw.dbg.dest_tlv)
  1425				goto out_free_fw;
  1426	
  1427			if (*pieces->dbg_dest_ver == 0) {
  1428				memcpy(drv->fw.dbg.dest_tlv, pieces->dbg_dest_tlv_v1,
  1429				       dbg_dest_size);
  1430			} else {
  1431				struct iwl_fw_dbg_dest_tlv_v1 *dest_tlv =
  1432					drv->fw.dbg.dest_tlv;
  1433	
  1434				dest_tlv->version = pieces->dbg_dest_tlv->version;
  1435				dest_tlv->monitor_mode =
  1436					pieces->dbg_dest_tlv->monitor_mode;
  1437				dest_tlv->size_power =
  1438					pieces->dbg_dest_tlv->size_power;
  1439				dest_tlv->wrap_count =
  1440					pieces->dbg_dest_tlv->wrap_count;
  1441				dest_tlv->write_ptr_reg =
  1442					pieces->dbg_dest_tlv->write_ptr_reg;
  1443				dest_tlv->base_shift =
  1444					pieces->dbg_dest_tlv->base_shift;
  1445				memcpy(dest_tlv->reg_ops,
  1446				       pieces->dbg_dest_tlv->reg_ops,
  1447				       sizeof(drv->fw.dbg.dest_tlv->reg_ops[0]) *
  1448				       drv->fw.dbg.n_dest_reg);
  1449	
  1450				/* In version 1 of the destination tlv, which is
  1451				 * relevant for internal buffer exclusively,
  1452				 * the base address is part of given with the length
  1453				 * of the buffer, and the size shift is give instead of
  1454				 * end shift. We now store these values in base_reg,
  1455				 * and end shift, and when dumping the data we'll
  1456				 * manipulate it for extracting both the length and
  1457				 * base address */
  1458				dest_tlv->base_reg = pieces->dbg_dest_tlv->cfg_reg;
  1459				dest_tlv->end_shift =
  1460					pieces->dbg_dest_tlv->size_shift;
  1461			}
  1462		}
  1463	
  1464		for (i = 0; i < ARRAY_SIZE(drv->fw.dbg.conf_tlv); i++) {
  1465			if (pieces->dbg_conf_tlv[i]) {
  1466				drv->fw.dbg.conf_tlv[i] =
  1467					kmemdup(pieces->dbg_conf_tlv[i],
  1468						pieces->dbg_conf_tlv_len[i],
  1469						GFP_KERNEL);
> 1470				if (!drv->fw.dbg_conf_tlv[i])
  1471					goto out_free_fw;
  1472			}
  1473		}
  1474	
  1475		memset(&trigger_tlv_sz, 0xff, sizeof(trigger_tlv_sz));
  1476	
  1477		trigger_tlv_sz[FW_DBG_TRIGGER_MISSED_BEACONS] =
  1478			sizeof(struct iwl_fw_dbg_trigger_missed_bcon);
  1479		trigger_tlv_sz[FW_DBG_TRIGGER_CHANNEL_SWITCH] = 0;
  1480		trigger_tlv_sz[FW_DBG_TRIGGER_FW_NOTIF] =
  1481			sizeof(struct iwl_fw_dbg_trigger_cmd);
  1482		trigger_tlv_sz[FW_DBG_TRIGGER_MLME] =
  1483			sizeof(struct iwl_fw_dbg_trigger_mlme);
  1484		trigger_tlv_sz[FW_DBG_TRIGGER_STATS] =
  1485			sizeof(struct iwl_fw_dbg_trigger_stats);
  1486		trigger_tlv_sz[FW_DBG_TRIGGER_RSSI] =
  1487			sizeof(struct iwl_fw_dbg_trigger_low_rssi);
  1488		trigger_tlv_sz[FW_DBG_TRIGGER_TXQ_TIMERS] =
  1489			sizeof(struct iwl_fw_dbg_trigger_txq_timer);
  1490		trigger_tlv_sz[FW_DBG_TRIGGER_TIME_EVENT] =
  1491			sizeof(struct iwl_fw_dbg_trigger_time_event);
  1492		trigger_tlv_sz[FW_DBG_TRIGGER_BA] =
  1493			sizeof(struct iwl_fw_dbg_trigger_ba);
  1494		trigger_tlv_sz[FW_DBG_TRIGGER_TDLS] =
  1495			sizeof(struct iwl_fw_dbg_trigger_tdls);
  1496	
  1497		for (i = 0; i < ARRAY_SIZE(drv->fw.dbg.trigger_tlv); i++) {
  1498			if (pieces->dbg_trigger_tlv[i]) {
  1499				/*
  1500				 * If the trigger isn't long enough, WARN and exit.
  1501				 * Someone is trying to debug something and he won't
  1502				 * be able to catch the bug he is trying to chase.
  1503				 * We'd better be noisy to be sure he knows what's
  1504				 * going on.
  1505				 */
  1506				if (WARN_ON(pieces->dbg_trigger_tlv_len[i] <
  1507					    (trigger_tlv_sz[i] +
  1508					     sizeof(struct iwl_fw_dbg_trigger_tlv))))
  1509					goto out_free_fw;
  1510				drv->fw.dbg.trigger_tlv_len[i] =
  1511					pieces->dbg_trigger_tlv_len[i];
  1512				drv->fw.dbg.trigger_tlv[i] =
  1513					kmemdup(pieces->dbg_trigger_tlv[i],
  1514						drv->fw.dbg.trigger_tlv_len[i],
  1515						GFP_KERNEL);
  1516				if (!drv->fw.dbg.trigger_tlv[i])
  1517					goto out_free_fw;
  1518			}
  1519		}
  1520	
  1521		/* Now that we can no longer fail, copy information */
  1522	
  1523		drv->fw.dbg.mem_tlv = pieces->dbg_mem_tlv;
  1524		pieces->dbg_mem_tlv = NULL;
  1525		drv->fw.dbg.n_mem_tlv = pieces->n_mem_tlv;
  1526	
  1527		/*
  1528		 * The (size - 16) / 12 formula is based on the information recorded
  1529		 * for each event, which is of mode 1 (including timestamp) for all
  1530		 * new microcodes that include this information.
  1531		 */
  1532		fw->init_evtlog_ptr = pieces->init_evtlog_ptr;
  1533		if (pieces->init_evtlog_size)
  1534			fw->init_evtlog_size = (pieces->init_evtlog_size - 16)/12;
  1535		else
  1536			fw->init_evtlog_size =
  1537				drv->trans->trans_cfg->base_params->max_event_log_size;
  1538		fw->init_errlog_ptr = pieces->init_errlog_ptr;
  1539		fw->inst_evtlog_ptr = pieces->inst_evtlog_ptr;
  1540		if (pieces->inst_evtlog_size)
  1541			fw->inst_evtlog_size = (pieces->inst_evtlog_size - 16)/12;
  1542		else
  1543			fw->inst_evtlog_size =
  1544				drv->trans->trans_cfg->base_params->max_event_log_size;
  1545		fw->inst_errlog_ptr = pieces->inst_errlog_ptr;
  1546	
  1547		/*
  1548		 * figure out the offset of chain noise reset and gain commands
  1549		 * base on the size of standard phy calibration commands table size
  1550		 */
  1551		if (fw->ucode_capa.standard_phy_calibration_size >
  1552		    IWL_MAX_PHY_CALIBRATE_TBL_SIZE)
  1553			fw->ucode_capa.standard_phy_calibration_size =
  1554				IWL_MAX_STANDARD_PHY_CALIBRATE_TBL_SIZE;
  1555	
  1556		/* We have our copies now, allow OS release its copies */
  1557		release_firmware(ucode_raw);
  1558	
  1559		mutex_lock(&iwlwifi_opmode_table_mtx);
  1560		switch (fw->type) {
  1561		case IWL_FW_DVM:
  1562			op = &iwlwifi_opmode_table[DVM_OP_MODE];
  1563			break;
  1564		default:
  1565			WARN(1, "Invalid fw type %d\n", fw->type);
  1566			/* fall through */
  1567		case IWL_FW_MVM:
  1568			op = &iwlwifi_opmode_table[MVM_OP_MODE];
  1569			break;
  1570		}
  1571	
  1572		IWL_INFO(drv, "loaded firmware version %s op_mode %s\n",
  1573			 drv->fw.fw_version, op->name);
  1574	
  1575		iwl_dbg_tlv_load_bin(drv->trans->dev, drv->trans);
  1576	
  1577		/* add this device to the list of devices using this op_mode */
  1578		list_add_tail(&drv->list, &op->drv);
  1579	
  1580		if (op->ops) {
  1581			drv->op_mode = _iwl_op_mode_start(drv, op);
  1582	
  1583			if (!drv->op_mode) {
  1584				mutex_unlock(&iwlwifi_opmode_table_mtx);
  1585				goto out_unbind;
  1586			}
  1587		} else {
  1588			load_module = true;
  1589		}
  1590		mutex_unlock(&iwlwifi_opmode_table_mtx);
  1591	
  1592		/*
  1593		 * Complete the firmware request last so that
  1594		 * a driver unbind (stop) doesn't run while we
  1595		 * are doing the start() above.
  1596		 */
  1597		complete(&drv->request_firmware_complete);
  1598	
  1599		/*
  1600		 * Load the module last so we don't block anything
  1601		 * else from proceeding if the module fails to load
  1602		 * or hangs loading.
  1603		 */
  1604		if (load_module) {
  1605			request_module("%s", op->name);
  1606	#ifdef CONFIG_IWLWIFI_OPMODE_MODULAR
  1607			if (err)
  1608				IWL_ERR(drv,
  1609					"failed to load module %s (error %d), is dynamic loading enabled?\n",
  1610					op->name, err);
  1611	#endif
  1612		}
  1613		goto free;
  1614	
  1615	 try_again:
  1616		/* try next, if any */
  1617		release_firmware(ucode_raw);
  1618		if (iwl_request_firmware(drv, false))
  1619			goto out_unbind;
  1620		goto free;
  1621	
  1622	 out_free_fw:
  1623		release_firmware(ucode_raw);
  1624	 out_unbind:
  1625		complete(&drv->request_firmware_complete);
  1626		device_release_driver(drv->trans->dev);
  1627	 free:
  1628		if (pieces) {
  1629			for (i = 0; i < ARRAY_SIZE(pieces->img); i++)
  1630				kfree(pieces->img[i].sec);
  1631			kfree(pieces->dbg_mem_tlv);
  1632			kfree(pieces);
  1633		}
  1634	}
  1635	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 17481 bytes --]

Re: [PATCH] iwlwifi: actually check allocated conf_tlv pointer

[Kalle Valo]

Chris Rorvick <chris@rorvick.com> writes:

> Commit 71bc0334a637 ("iwlwifi: check allocated pointer when allocating
> conf_tlvs") attempted to fix a typoe introduced by commit 17b809c9b22e
> ("iwlwifi: dbg: move debug data to a struct") but does not implement the
> check correctly.
>
> Tweeted-by: @grsecurity
> Signed-off-by: Chris Rorvick <chris@rorvick.com>

I'll add:

Fixes: 71bc0334a637 ("iwlwifi: check allocated pointer when allocating conf_tlvs")

> ---
> In this wasn't picked up?

Luca, can I take this directly?

-- 
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH] iwlwifi: actually check allocated conf_tlv pointer

[Luca Coelho]

On Sun, 2020-04-05 at 11:44 +0300, Kalle Valo wrote:
> Chris Rorvick <chris@rorvick.com> writes:
> 
> > Commit 71bc0334a637 ("iwlwifi: check allocated pointer when allocating
> > conf_tlvs") attempted to fix a typoe introduced by commit 17b809c9b22e
> > ("iwlwifi: dbg: move debug data to a struct") but does not implement the
> > check correctly.
> > 
> > Tweeted-by: @grsecurity
> > Signed-off-by: Chris Rorvick <chris@rorvick.com>
> 
> I'll add:
> 
> Fixes: 71bc0334a637 ("iwlwifi: check allocated pointer when allocating conf_tlvs")
> 
> > ---
> > In this wasn't picked up?
> 
> Luca, can I take this directly?

Yes, please take it directly.  This can happen in OOM situations and,
when it does, we will potentially try to dereference a NULL pointer.

Thanks!

--
Cheers,
Luca.

Re: [PATCH] iwlwifi: actually check allocated conf_tlv pointer

[Kalle Valo]

Luca Coelho <luca@coelho.fi> writes:

> On Sun, 2020-04-05 at 11:44 +0300, Kalle Valo wrote:
>> Chris Rorvick <chris@rorvick.com> writes:
>> 
>> > Commit 71bc0334a637 ("iwlwifi: check allocated pointer when allocating
>> > conf_tlvs") attempted to fix a typoe introduced by commit 17b809c9b22e
>> > ("iwlwifi: dbg: move debug data to a struct") but does not implement the
>> > check correctly.
>> > 
>> > Tweeted-by: @grsecurity
>> > Signed-off-by: Chris Rorvick <chris@rorvick.com>
>> 
>> I'll add:
>> 
>> Fixes: 71bc0334a637 ("iwlwifi: check allocated pointer when allocating conf_tlvs")
>> 
>> > ---
>> > In this wasn't picked up?
>> 
>> Luca, can I take this directly?
>
> Yes, please take it directly.

Ok, assigned it to me in patchwork.

> This can happen in OOM situations and, when it does, we will
> potentially try to dereference a NULL pointer.

I'll add this to the commit log.

-- 
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH] iwlwifi: actually check allocated conf_tlv pointer

[Kalle Valo]

Chris Rorvick <chris@rorvick.com> wrote:

> Commit 71bc0334a637 ("iwlwifi: check allocated pointer when allocating
> conf_tlvs") attempted to fix a typoe introduced by commit 17b809c9b22e
> ("iwlwifi: dbg: move debug data to a struct") but does not implement the
> check correctly.
> 
> This can happen in OOM situations and, when it does, we will potentially try to
> dereference a NULL pointer.
> 
> Tweeted-by: @grsecurity
> Signed-off-by: Chris Rorvick <chris@rorvick.com>

Fails to build, please rebase on top of wireless-drivers.

drivers/net/wireless/intel/iwlwifi/iwl-drv.c: In function 'iwl_req_fw_callback':
drivers/net/wireless/intel/iwlwifi/iwl-drv.c:1470:16: error: 'struct iwl_fw' has no member named 'dbg_conf_tlv'
    if (!drv->fw.dbg_conf_tlv[i])
                ^
make[5]: *** [drivers/net/wireless/intel/iwlwifi/iwl-drv.o] Error 1
make[5]: *** Waiting for unfinished jobs....
make[4]: *** [drivers/net/wireless/intel/iwlwifi] Error 2
make[3]: *** [drivers/net/wireless/intel] Error 2
make[2]: *** [drivers/net/wireless] Error 2
make[1]: *** [drivers/net] Error 2
make[1]: *** Waiting for unfinished jobs....
make: *** [drivers] Error 2

Patch set to Changes Requested.

-- 
https://patchwork.kernel.org/patch/11470125/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH] iwlwifi: actually check allocated conf_tlv pointer

[Sedat Dilek]

On Mon, Apr 6, 2020 at 4:11 PM Kalle Valo <kvalo@codeaurora.org> wrote:
>
> Chris Rorvick <chris@rorvick.com> wrote:
>
> > Commit 71bc0334a637 ("iwlwifi: check allocated pointer when allocating
> > conf_tlvs") attempted to fix a typoe introduced by commit 17b809c9b22e
> > ("iwlwifi: dbg: move debug data to a struct") but does not implement the
> > check correctly.
> >
> > This can happen in OOM situations and, when it does, we will potentially try to
> > dereference a NULL pointer.
> >
> > Tweeted-by: @grsecurity
> > Signed-off-by: Chris Rorvick <chris@rorvick.com>
>
> Fails to build, please rebase on top of wireless-drivers.
>
> drivers/net/wireless/intel/iwlwifi/iwl-drv.c: In function 'iwl_req_fw_callback':
> drivers/net/wireless/intel/iwlwifi/iwl-drv.c:1470:16: error: 'struct iwl_fw' has no member named 'dbg_conf_tlv'
>     if (!drv->fw.dbg_conf_tlv[i])
>                 ^
> make[5]: *** [drivers/net/wireless/intel/iwlwifi/iwl-drv.o] Error 1
> make[5]: *** Waiting for unfinished jobs....
> make[4]: *** [drivers/net/wireless/intel/iwlwifi] Error 2
> make[3]: *** [drivers/net/wireless/intel] Error 2
> make[2]: *** [drivers/net/wireless] Error 2
> make[1]: *** [drivers/net] Error 2
> make[1]: *** Waiting for unfinished jobs....
> make: *** [drivers] Error 2
>

Should be:

$ git diff
diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
index 0481796f75bc..c24350222133 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
@@ -1467,7 +1467,7 @@ static void iwl_req_fw_callback(const struct
firmware *ucode_raw, void *context)
                                kmemdup(pieces->dbg_conf_tlv[i],
                                        pieces->dbg_conf_tlv_len[i],
                                        GFP_KERNEL);
-                       if (!pieces->dbg_conf_tlv[i])
+                       if (!drv->fw.dbg.conf_tlv[i])
                                goto out_free_fw;
                }
        }

"fw.dbg.conf" with a dot not underscore.

- Sedat -



> Patch set to Changes Requested.
>
> --
> https://patchwork.kernel.org/patch/11470125/
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH] iwlwifi: actually check allocated conf_tlv pointer

[Sedat Dilek]

On Mon, Apr 6, 2020 at 9:53 PM Sedat Dilek <sedat.dilek@gmail.com> wrote:
>
> On Mon, Apr 6, 2020 at 4:11 PM Kalle Valo <kvalo@codeaurora.org> wrote:
> >
> > Chris Rorvick <chris@rorvick.com> wrote:
> >
> > > Commit 71bc0334a637 ("iwlwifi: check allocated pointer when allocating
> > > conf_tlvs") attempted to fix a typoe introduced by commit 17b809c9b22e
> > > ("iwlwifi: dbg: move debug data to a struct") but does not implement the
> > > check correctly.
> > >
> > > This can happen in OOM situations and, when it does, we will potentially try to
> > > dereference a NULL pointer.
> > >
> > > Tweeted-by: @grsecurity
> > > Signed-off-by: Chris Rorvick <chris@rorvick.com>
> >
> > Fails to build, please rebase on top of wireless-drivers.
> >
> > drivers/net/wireless/intel/iwlwifi/iwl-drv.c: In function 'iwl_req_fw_callback':
> > drivers/net/wireless/intel/iwlwifi/iwl-drv.c:1470:16: error: 'struct iwl_fw' has no member named 'dbg_conf_tlv'
> >     if (!drv->fw.dbg_conf_tlv[i])
> >                 ^
> > make[5]: *** [drivers/net/wireless/intel/iwlwifi/iwl-drv.o] Error 1
> > make[5]: *** Waiting for unfinished jobs....
> > make[4]: *** [drivers/net/wireless/intel/iwlwifi] Error 2
> > make[3]: *** [drivers/net/wireless/intel] Error 2
> > make[2]: *** [drivers/net/wireless] Error 2
> > make[1]: *** [drivers/net] Error 2
> > make[1]: *** Waiting for unfinished jobs....
> > make: *** [drivers] Error 2
> >
>
> Should be:
>
> $ git diff
> diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
> b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
> index 0481796f75bc..c24350222133 100644
> --- a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
> +++ b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
> @@ -1467,7 +1467,7 @@ static void iwl_req_fw_callback(const struct
> firmware *ucode_raw, void *context)
>                                 kmemdup(pieces->dbg_conf_tlv[i],
>                                         pieces->dbg_conf_tlv_len[i],
>                                         GFP_KERNEL);

Maybe this diff is clearer:

$ diff iwlwifi-actually-check-allocated-conf_tlv-pointer.patch
iwlwifi-actually-check-allocated-conf_tlv-pointer-v2-dileks.patch
95a96
> Fixes: 71bc0334a637 ("iwlwifi: check allocated pointer when allocating conf_tlvs")
99c100,104
< In this wasn't picked up?
---
>
> [ v1->v2:
>   - Fix typo s/fw.dbg_conf_tlv/fw.dbg.conf_tlv
>   - Add Fixes tag as suggested by Kalle
> -dileks ]
115c120
< +                     if (!drv->fw.dbg_conf_tlv[i])
---
> +                     if (!drv->fw.dbg.conf_tlv[i])

Tested on top of Linux v5.6.3.

- Sedat -

Re: [PATCH] iwlwifi: actually check allocated conf_tlv pointer

[Sedat Dilek]

[-- Attachment #1: Type: text/plain, Size: 1587 bytes --]

On Sun, Apr 5, 2020 at 11:14 AM Kalle Valo <kvalo@codeaurora.org> wrote:
>
> Luca Coelho <luca@coelho.fi> writes:
>
> > On Sun, 2020-04-05 at 11:44 +0300, Kalle Valo wrote:
> >> Chris Rorvick <chris@rorvick.com> writes:
> >>
> >> > Commit 71bc0334a637 ("iwlwifi: check allocated pointer when allocating
> >> > conf_tlvs") attempted to fix a typoe introduced by commit 17b809c9b22e
> >> > ("iwlwifi: dbg: move debug data to a struct") but does not implement the
> >> > check correctly.
> >> >
> >> > Tweeted-by: @grsecurity
> >> > Signed-off-by: Chris Rorvick <chris@rorvick.com>
> >>
> >> I'll add:
> >>
> >> Fixes: 71bc0334a637 ("iwlwifi: check allocated pointer when allocating conf_tlvs")
> >>
> >> > ---
> >> > In this wasn't picked up?
> >>
> >> Luca, can I take this directly?
> >
> > Yes, please take it directly.
>
> Ok, assigned it to me in patchwork.
>
> > This can happen in OOM situations and, when it does, we will
> > potentially try to dereference a NULL pointer.
>
> I'll add this to the commit log.
>

Hi,

Friendly ping.

Any progress on this?

This patch seems not have landed in Linux v5.7-rc1.

$ head -5 Makefile
# SPDX-License-Identifier: GPL-2.0
VERSION = 5
PATCHLEVEL = 7
SUBLEVEL = 0
EXTRAVERSION = -rc1

$ LC_ALL=C git apply --check --verbose
../patches/iwlwifi-fixes-5.6/iwlwifi-actually-check-allocated-conf_tlv-pointer-v2-dileks.patch
Checking patch drivers/net/wireless/intel/iwlwifi/iwl-drv.c...

I have attached my v2 which I have tested on top of Linux v5.6.3.

Feel free to add my...

Tested-by: Sedat Dilek <sedat.dilek@gmail.com>

Regards,
- Sedat -

[-- Attachment #2: iwlwifi-actually-check-allocated-conf_tlv-pointer-v2-dileks.patch --]
[-- Type: text/x-patch, Size: 5913 bytes --]

From patchwork Thu Apr  2 05:02:19 2020
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
X-Patchwork-Submitter: Chris Rorvick <chris@rorvick.com>
X-Patchwork-Id: 11470125
X-Patchwork-Delegate: kvalo@adurom.com
Return-Path: <SRS0=piom=5S=vger.kernel.org=linux-wireless-owner@kernel.org>
Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org
 [172.30.200.123])
	by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 4918115AB
	for <patchwork-linux-wireless@patchwork.kernel.org>;
 Thu,  2 Apr 2020 05:40:22 +0000 (UTC)
Received: from vger.kernel.org (vger.kernel.org [209.132.180.67])
	by mail.kernel.org (Postfix) with ESMTP id 26CF720784
	for <patchwork-linux-wireless@patchwork.kernel.org>;
 Thu,  2 Apr 2020 05:40:22 +0000 (UTC)
Authentication-Results: mail.kernel.org;
	dkim=fail reason="signature verification failed" (2048-bit key)
 header.d=gmail.com header.i=@gmail.com header.b="oGgRiCyG"
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1727746AbgDBFkR (ORCPT
        <rfc822;patchwork-linux-wireless@patchwork.kernel.org>);
        Thu, 2 Apr 2020 01:40:17 -0400
Received: from mail-qk1-f193.google.com ([209.85.222.193]:34234 "EHLO
        mail-qk1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1726201AbgDBFkR (ORCPT
        <rfc822;linux-wireless@vger.kernel.org>);
        Thu, 2 Apr 2020 01:40:17 -0400
Received: by mail-qk1-f193.google.com with SMTP id i6so2814149qke.1;
        Wed, 01 Apr 2020 22:40:16 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
        d=gmail.com; s=20161025;
        h=sender:from:to:cc:subject:date:message-id:mime-version
         :content-transfer-encoding;
        bh=Hj9hqNcME2oIKIw+5Vx3xGDXGbaFZ4ovP+thufGAVmU=;
        b=oGgRiCyGM1NttzIOzX4lwfWaLVWlVNJVxi3p2J32f8r5BrPNarW8vx/Yl6iLZg5HU/
         y1n7ZlChgcMzf6CBkZqbMoDBLXJcQzKXki+ZUF7JoXFN4UXXS8qJ4XMG1Dy5hJIP3D2q
         PxIWu5Jz96mjpP+jTJdf8y6ohWWqVQYGRE+1Buh1xTTha5aNmdttUQo7vhVy+mQC//Xs
         LEuGp+m+fJGeAswwrLaJzN9iuSErM4LRexHOGPl21AVS1fxbUL0yQWNG/9NYEI/wx1B6
         sLOCt5cU4xBWmc99Zmu9lpWLpu4MQRIbmJsSeB8vI+u6Zhyi9W1GAygUEPh5rP6Dw1iu
         YEvw==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
        d=1e100.net; s=20161025;
        h=x-gm-message-state:sender:from:to:cc:subject:date:message-id
         :mime-version:content-transfer-encoding;
        bh=Hj9hqNcME2oIKIw+5Vx3xGDXGbaFZ4ovP+thufGAVmU=;
        b=Smif+8RHLKd1nWQ1GDTERfSIhNR7k4Z7JHxwVtF7WlQghN6KbKR3gkPWuGkSbacT3/
         4DYz127WT4bSInQrSv+qtk+X12RjcBiMlovv5pHNelKD6i1a1aP9xZFo0LTwT860HKx1
         y6qxIHvObx/GheS4j0u3ogus0uNzEvTnlIptNQhA3ifwvQNbDh8CuJWaeQVvNB29si1v
         7kNV8ivhLPLpGA99IAjgA08wgROQD68QC/SpEahxG2LzEG0dITRMUeFoHAq/5YYBuBy5
         8OxEfsR0G3SqdqLYl8qnuRFFpixh++N2XKpDhqP23Y25U3UZfVRYixxpVTGCqJhqxKWQ
         FEQA==
X-Gm-Message-State: AGi0PubsIirzpTp/gtfpoocLpGEqUHWZ6H4oC2JT+fOsEwWJEP7mCPKO
        GpUyv8R62QuZ9JFioWVUj+u9Yktv
X-Google-Smtp-Source: 
 APiQypKYQzwTCauSCZsNzFwP7pSAbf43FjGqHiBn/lCnEhAbO+zIU1RyY8RLX9MuztxrJHSKsfzyeg==
X-Received: by 2002:a37:4c4d:: with SMTP id z74mr1842987qka.53.1585806015666;
        Wed, 01 Apr 2020 22:40:15 -0700 (PDT)
Received: from localhost (c-73-74-7-9.hsd1.il.comcast.net. [73.74.7.9])
        by smtp.gmail.com with ESMTPSA id
 t140sm2911459qke.48.2020.04.01.22.40.13
        (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);
        Wed, 01 Apr 2020 22:40:14 -0700 (PDT)
Received: from localhost (localhost [127.0.0.1])
        by localhost (8.15.2/8.14.9) with ESMTP id 0325eAOc005904;
        Thu, 2 Apr 2020 00:40:12 -0500
Received: (from chris@localhost)
        by localhost (8.15.2/8.15.2/Submit) id 03254KY3004887;
        Thu, 2 Apr 2020 00:04:20 -0500
From: Chris Rorvick <chris@rorvick.com>
To: linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
Cc: Chris Rorvick <chris@rorvick.com>,
        Johannes Berg <johannes.berg@intel.com>,
        Emmanuel Grumbach <emmanuel.grumbach@intel.com>,
        Luca Coelho <luciano.coelho@intel.com>,
        Intel Linux Wireless <linuxwifi@intel.com>,
        Kalle Valo <kvalo@codeaurora.org>,
        "David S. Miller" <davem@davemloft.net>
Subject: [PATCH] iwlwifi: actually check allocated conf_tlv pointer
Date: Thu,  2 Apr 2020 00:02:19 -0500
Message-Id: <20200402050219.4842-1-chris@rorvick.com>
X-Mailer: git-send-email 2.25.0
MIME-Version: 1.0
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-wireless.vger.kernel.org>
X-Mailing-List: linux-wireless@vger.kernel.org

Commit 71bc0334a637 ("iwlwifi: check allocated pointer when allocating
conf_tlvs") attempted to fix a typoe introduced by commit 17b809c9b22e
("iwlwifi: dbg: move debug data to a struct") but does not implement the
check correctly.

Fixes: 71bc0334a637 ("iwlwifi: check allocated pointer when allocating conf_tlvs")
Tweeted-by: @grsecurity
Signed-off-by: Chris Rorvick <chris@rorvick.com>
---

[ v1->v2:
  - Fix typo s/fw.dbg_conf_tlv/fw.dbg.conf_tlv
  - Add Fixes tag as suggested by Kalle
-dileks ]

 drivers/net/wireless/intel/iwlwifi/iwl-drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
index ff52e69c1c80..a37f330e7bd4 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
@@ -1465,11 +1465,11 @@ static void iwl_req_fw_callback(const struct firmware *ucode_raw, void *context)
 		if (pieces->dbg_conf_tlv[i]) {
 			drv->fw.dbg.conf_tlv[i] =
 				kmemdup(pieces->dbg_conf_tlv[i],
 					pieces->dbg_conf_tlv_len[i],
 					GFP_KERNEL);
-			if (!pieces->dbg_conf_tlv[i])
+			if (!drv->fw.dbg.conf_tlv[i])
 				goto out_free_fw;
 		}
 	}
 
 	memset(&trigger_tlv_sz, 0xff, sizeof(trigger_tlv_sz));