* [PATCH] iwlwifi: actually check allocated conf_tlv pointer @ 2020-04-02 5:02 Chris Rorvick 2020-04-02 9:41 ` kbuild test robot ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: Chris Rorvick @ 2020-04-02 5:02 UTC (permalink / raw) To: linux-wireless, netdev, linux-kernel Cc: Chris Rorvick, Johannes Berg, Emmanuel Grumbach, Luca Coelho, Intel Linux Wireless, Kalle Valo, David S. Miller 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] iwlwifi: actually check allocated conf_tlv pointer 2020-04-02 5:02 [PATCH] iwlwifi: actually check allocated conf_tlv pointer Chris Rorvick @ 2020-04-02 9:41 ` kbuild test robot 2020-04-05 8:44 ` Kalle Valo 2020-04-06 14:10 ` Kalle Valo 2 siblings, 0 replies; 9+ messages in thread From: kbuild test robot @ 2020-04-02 9:41 UTC (permalink / raw) To: kbuild-all [-- 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 --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] iwlwifi: actually check allocated conf_tlv pointer 2020-04-02 5:02 [PATCH] iwlwifi: actually check allocated conf_tlv pointer Chris Rorvick 2020-04-02 9:41 ` kbuild test robot @ 2020-04-05 8:44 ` Kalle Valo 2020-04-05 8:51 ` Luca Coelho 2020-04-06 14:10 ` Kalle Valo 2 siblings, 1 reply; 9+ messages in thread From: Kalle Valo @ 2020-04-05 8:44 UTC (permalink / raw) To: Chris Rorvick Cc: linux-wireless, netdev, linux-kernel, Johannes Berg, Emmanuel Grumbach, Luca Coelho, Intel Linux Wireless, David S. Miller 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] iwlwifi: actually check allocated conf_tlv pointer 2020-04-05 8:44 ` Kalle Valo @ 2020-04-05 8:51 ` Luca Coelho 2020-04-05 9:13 ` Kalle Valo 0 siblings, 1 reply; 9+ messages in thread From: Luca Coelho @ 2020-04-05 8:51 UTC (permalink / raw) To: Kalle Valo, Chris Rorvick Cc: linux-wireless, netdev, linux-kernel, Johannes Berg, Emmanuel Grumbach, Intel Linux Wireless, David S. Miller 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. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] iwlwifi: actually check allocated conf_tlv pointer 2020-04-05 8:51 ` Luca Coelho @ 2020-04-05 9:13 ` Kalle Valo 2020-04-14 12:14 ` Sedat Dilek 0 siblings, 1 reply; 9+ messages in thread From: Kalle Valo @ 2020-04-05 9:13 UTC (permalink / raw) To: Luca Coelho Cc: Chris Rorvick, linux-wireless, netdev, linux-kernel, Johannes Berg, Emmanuel Grumbach, Intel Linux Wireless, David S. Miller 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] iwlwifi: actually check allocated conf_tlv pointer 2020-04-05 9:13 ` Kalle Valo @ 2020-04-14 12:14 ` Sedat Dilek 0 siblings, 0 replies; 9+ messages in thread From: Sedat Dilek @ 2020-04-14 12:14 UTC (permalink / raw) To: Kalle Valo Cc: Luca Coelho, Chris Rorvick, linux-wireless, netdev, linux-kernel, Johannes Berg, Emmanuel Grumbach, Intel Linux Wireless, David S. Miller [-- 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)); ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] iwlwifi: actually check allocated conf_tlv pointer 2020-04-02 5:02 [PATCH] iwlwifi: actually check allocated conf_tlv pointer Chris Rorvick 2020-04-02 9:41 ` kbuild test robot 2020-04-05 8:44 ` Kalle Valo @ 2020-04-06 14:10 ` Kalle Valo 2020-04-06 19:53 ` Sedat Dilek 2 siblings, 1 reply; 9+ messages in thread From: Kalle Valo @ 2020-04-06 14:10 UTC (permalink / raw) To: Chris Rorvick Cc: linux-wireless, netdev, linux-kernel, Chris Rorvick, Johannes Berg, Emmanuel Grumbach, Luca Coelho, Intel Linux Wireless, David S. Miller 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] iwlwifi: actually check allocated conf_tlv pointer 2020-04-06 14:10 ` Kalle Valo @ 2020-04-06 19:53 ` Sedat Dilek 2020-04-08 9:07 ` Sedat Dilek 0 siblings, 1 reply; 9+ messages in thread From: Sedat Dilek @ 2020-04-06 19:53 UTC (permalink / raw) To: Kalle Valo Cc: Chris Rorvick, linux-wireless, netdev, linux-kernel, Johannes Berg, Emmanuel Grumbach, Luca Coelho, Intel Linux Wireless, David S. Miller 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] iwlwifi: actually check allocated conf_tlv pointer 2020-04-06 19:53 ` Sedat Dilek @ 2020-04-08 9:07 ` Sedat Dilek 0 siblings, 0 replies; 9+ messages in thread From: Sedat Dilek @ 2020-04-08 9:07 UTC (permalink / raw) To: Kalle Valo Cc: Chris Rorvick, linux-wireless, netdev, linux-kernel, Johannes Berg, Emmanuel Grumbach, Luca Coelho, Intel Linux Wireless, David S. Miller 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 - ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-04-14 12:14 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-04-02 5:02 [PATCH] iwlwifi: actually check allocated conf_tlv pointer Chris Rorvick 2020-04-02 9:41 ` kbuild test robot 2020-04-05 8:44 ` Kalle Valo 2020-04-05 8:51 ` Luca Coelho 2020-04-05 9:13 ` Kalle Valo 2020-04-14 12:14 ` Sedat Dilek 2020-04-06 14:10 ` Kalle Valo 2020-04-06 19:53 ` Sedat Dilek 2020-04-08 9:07 ` Sedat Dilek
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.