[patch -next] qed: potential overflow in qed_cxt_src_t2_alloc()
In the current code "ent_per_page" could be more than "conn_num" making "conn_num" negative after the subtraction. In the next iteration through the loop then the negative is treated as a very high positive meaning we don't put a limit on "ent_num". It could lead to memory corruption. Fixes: dbb799c39717 ('qed: Initialize hardware for new protocols') Signed-off-by: Dan Carpenter diff --git a/drivers/net/ethernet/qlogic/qed/qed_cxt.c b/drivers/net/ethernet/qlogic/qed/qed_cxt.c index d85b7ba..1c35f37 100644 --- a/drivers/net/ethernet/qlogic/qed/qed_cxt.c +++ b/drivers/net/ethernet/qlogic/qed/qed_cxt.c @@ -850,7 +850,7 @@ static int qed_cxt_src_t2_alloc(struct qed_hwfn *p_hwfn) val = 0; entries[j].next = cpu_to_be64(val); - conn_num -= ent_per_page; + conn_num -= ent_num; } return 0;
[patch] rtlwifi: rtl818x: silence uninitialized variable warning
What about if "rtlphy->pwrgroup_cnt" is 2? In that case we would use an uninitialized "chnlgroup" variable and probably crash. Maybe that can't happen for some reason which is not obvious but in that case this patch is harmless. Setting it to zero seems like a standard default in the surrounding code so it's probably fine here as well. Signed-off-by: Dan Carpenter diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192se/rf.c b/drivers/net/wireless/realtek/rtlwifi/rtl8192se/rf.c index 78a81c1..9475aa2 100644 --- a/drivers/net/wireless/realtek/rtlwifi/rtl8192se/rf.c +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192se/rf.c @@ -208,8 +208,7 @@ static void _rtl92s_get_txpower_writeval_byregulatory(struct ieee80211_hw *hw, "Realtek regulatory, 40MHz, writeval = 0x%x\n", writeval); } else { - if (rtlphy->pwrgroup_cnt == 1) - chnlgroup = 0; + chnlgroup = 0; if (rtlphy->pwrgroup_cnt >= 3) { if (chnl <= 3)
[patch] usbnet/smsc75xx: silence uninitialized variable warning
If the fn() calls fail then "buf" is uninitialized. Just return early in that situation. Signed-off-by: Dan Carpenter diff --git a/drivers/net/usb/smsc75xx.c b/drivers/net/usb/smsc75xx.c index c369db9..9af9799 100644 --- a/drivers/net/usb/smsc75xx.c +++ b/drivers/net/usb/smsc75xx.c @@ -99,9 +99,11 @@ static int __must_check __smsc75xx_read_reg(struct usbnet *dev, u32 index, ret = fn(dev, USB_VENDOR_REQUEST_READ_REGISTER, USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE, 0, index, &buf, 4); - if (unlikely(ret < 0)) + if (unlikely(ret < 0)) { netdev_warn(dev->net, "Failed to read reg index 0x%08x: %d\n", index, ret); + return ret; + } le32_to_cpus(&buf); *data = buf;
[patch] usbnet: smsc95xx: silence an uninitialized variable warning
If the call to fn() fails then "buf" is uninitialized. Just return the error code in that case. Signed-off-by: Dan Carpenter diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c index 2edc2bc..d9d2806 100644 --- a/drivers/net/usb/smsc95xx.c +++ b/drivers/net/usb/smsc95xx.c @@ -92,9 +92,11 @@ static int __must_check __smsc95xx_read_reg(struct usbnet *dev, u32 index, ret = fn(dev, USB_VENDOR_REQUEST_READ_REGISTER, USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE, 0, index, &buf, 4); - if (unlikely(ret < 0)) + if (unlikely(ret < 0)) { netdev_warn(dev->net, "Failed to read reg index 0x%08x: %d\n", index, ret); + return ret; + } le32_to_cpus(&buf); *data = buf;
[patch 1/2] netxen: fix error handling in netxen_get_flash_block()
My static checker complained that "v" can be used unintialized if netxen_rom_fast_read() returns -EIO. That function never actually returns -1. Signed-off-by: Dan Carpenter diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c index db80eb1..a320541 100644 --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c @@ -1015,20 +1015,24 @@ static int netxen_get_flash_block(struct netxen_adapter *adapter, int base, { int i, v, addr; __le32 *ptr32; + int ret; addr = base; ptr32 = buf; for (i = 0; i < size / sizeof(u32); i++) { - if (netxen_rom_fast_read(adapter, addr, &v) == -1) - return -1; + ret = netxen_rom_fast_read(adapter, addr, &v); + if (ret) + return ret; + *ptr32 = cpu_to_le32(v); ptr32++; addr += sizeof(u32); } if ((char *)buf + size > (char *)ptr32) { __le32 local; - if (netxen_rom_fast_read(adapter, addr, &v) == -1) - return -1; + ret = netxen_rom_fast_read(adapter, addr, &v); + if (ret) + return ret; local = cpu_to_le32(v); memcpy(ptr32, &local, (char *)buf + size - (char *)ptr32); }
[patch] i40e: fix an uninitialized variable bug
We removed this initialization but it is required. Let's put it back. Fixes: 895106a577c4 ('i40e: trivial fixes') Signed-off-by: Dan Carpenter diff --git a/drivers/net/ethernet/intel/i40e/i40e_hmc.c b/drivers/net/ethernet/intel/i40e/i40e_hmc.c index 5ebe12d..a7c7b1d 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_hmc.c +++ b/drivers/net/ethernet/intel/i40e/i40e_hmc.c @@ -49,7 +49,7 @@ i40e_status i40e_add_sd_table_entry(struct i40e_hw *hw, struct i40e_hmc_sd_entry *sd_entry; bool dma_mem_alloc_done = false; struct i40e_dma_mem mem; - i40e_status ret_code; + i40e_status ret_code = I40E_SUCCESS; u64 alloc_len; if (NULL == hmc_info->sd_table.sd_entry) {
[patch] netxen: netxen_rom_fast_read() doesn't return -1
The error handling is broken here. netxen_rom_fast_read() returns zero on success and -EIO on error. It never returns -1. Signed-off-by: Dan Carpenter diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c index fd362b6..9c6eed9 100644 --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c @@ -852,7 +852,8 @@ netxen_check_options(struct netxen_adapter *adapter) ptr32 = (__le32 *)&serial_num; offset = NX_FW_SERIAL_NUM_OFFSET; for (i = 0; i < 8; i++) { - if (netxen_rom_fast_read(adapter, offset, &val) == -1) { + err = netxen_rom_fast_read(adapter, offset, &val); + if (err) { dev_err(&pdev->dev, "error reading board info\n"); adapter->driver_mismatch = 1; return;
[patch] qede: uninitialized variable in qede_start_xmit()
"data_split" was never set to false. It's just uninitialized. Fixes: 2950219d87b0 ('qede: Add basic network device support') Signed-off-by: Dan Carpenter diff --git a/drivers/net/ethernet/qlogic/qede/qede_main.c b/drivers/net/ethernet/qlogic/qede/qede_main.c index 60a61c3..d98f35e 100644 --- a/drivers/net/ethernet/qlogic/qede/qede_main.c +++ b/drivers/net/ethernet/qlogic/qede/qede_main.c @@ -429,7 +429,7 @@ netdev_tx_t qede_start_xmit(struct sk_buff *skb, u8 xmit_type; u16 idx; u16 hlen; - bool data_split; + bool data_split = false; /* Get tx-queue context and netdev index */ txq_index = skb_get_queue_mapping(skb);
[patch 2/2] netxen: reversed condition in netxen_nic_set_link_parameters()
My static checker complains that we are using "autoneg" without initializing it. The problem is the ->phy_read() condition is reversed so we only set this on error instead of success. Signed-off-by: Dan Carpenter diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c index a320541..2b10f1b 100644 --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c @@ -1944,7 +1944,7 @@ void netxen_nic_set_link_parameters(struct netxen_adapter *adapter) if (adapter->phy_read && adapter->phy_read(adapter, NETXEN_NIU_GB_MII_MGMT_ADDR_AUTONEG, - &autoneg) != 0) + &autoneg) == 0) adapter->link_autoneg = autoneg; } else goto link_down;
[patch -mainline] qlcnic: potential NULL dereference in qlcnic_83xx_get_minidump_template()
If qlcnic_fw_cmd_get_minidump_temp() fails then "fw_dump->tmpl_hdr" is NULL or possibly freed. It can lead to an oops later. Fixes: d01a6d3c8ae1 ('qlcnic: Add support to enable capability to extend minidump for iSCSI') Signed-off-by: Dan Carpenter diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_minidump.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_minidump.c index cda9e60..0844b7c 100644 --- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_minidump.c +++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_minidump.c @@ -1417,6 +1417,7 @@ void qlcnic_83xx_get_minidump_template(struct qlcnic_adapter *adapter) struct qlcnic_fw_dump *fw_dump = &ahw->fw_dump; struct pci_dev *pdev = adapter->pdev; bool extended = false; + int ret; prev_version = adapter->fw_version; current_version = qlcnic_83xx_get_fw_version(adapter); @@ -1427,8 +1428,11 @@ void qlcnic_83xx_get_minidump_template(struct qlcnic_adapter *adapter) if (qlcnic_83xx_md_check_extended_dump_capability(adapter)) extended = !qlcnic_83xx_extend_md_capab(adapter); - if (!qlcnic_fw_cmd_get_minidump_temp(adapter)) - dev_info(&pdev->dev, "Supports FW dump capability\n"); + ret = qlcnic_fw_cmd_get_minidump_temp(adapter); + if (ret) + return; + + dev_info(&pdev->dev, "Supports FW dump capability\n"); /* Once we have minidump template with extended iSCSI dump * capability, update the minidump capture mask to 0x1f as
re: iwlwifi: mvm: add reorder buffer per queue
Hello Sara Sharon, The patch b915c10174fb: "iwlwifi: mvm: add reorder buffer per queue" from Mar 23, 2016, leads to the following static checker warnings: drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c:912 iwl_mvm_rx_mpdu_mq() error: potential NULL dereference 'sta'. drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c:912 iwl_mvm_rx_mpdu_mq() error: we previously assumed 'sta' could be null (see line 796) drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c 779 780 if (le16_to_cpu(desc->status) & IWL_RX_MPDU_STATUS_SRC_STA_FOUND) { 781 u8 id = desc->sta_id_flags & IWL_RX_MPDU_SIF_STA_ID_MASK; 782 783 if (!WARN_ON_ONCE(id >= IWL_MVM_STATION_COUNT)) { 784 sta = rcu_dereference(mvm->fw_id_to_mac_id[id]); 785 if (IS_ERR(sta)) 786 sta = NULL; ^^^ Assigned to NULL here. 787 } 788 } else if (!is_multicast_ether_addr(hdr->addr2)) { 789 /* 790 * This is fine since we prevent two stations with the same 791 * address from being added. 792 */ 793 sta = ieee80211_find_sta_by_ifaddr(mvm->hw, hdr->addr2, NULL); 794 } 795 796 if (sta) { ^^^ NULL here. 797 struct iwl_mvm_sta *mvmsta = iwl_mvm_sta_from_mac80211(sta); 798 u8 baid = (u8)((le32_to_cpu(desc->reorder_data) & 799 IWL_RX_MPDU_REORDER_BAID_MASK) >> 800 IWL_RX_MPDU_REORDER_BAID_SHIFT); [ snip ] 909 /* TODO: PHY info - gscan */ 910 911 iwl_mvm_create_skb(skb, hdr, len, crypt_len, rxb); 912 if (!iwl_mvm_reorder(mvm, napi, queue, sta, skb, desc)) ^^^ New unchecked dereference inside the function call. 913 iwl_mvm_pass_packet_to_mac80211(mvm, napi, skb, queue, sta); 914 rcu_read_unlock(); 915 } regards, dan carpenter
re: drivers: net: xgene: fix statistics counters race condition
Hello Iyappan Subramanian, The patch 3bb502f83080: "drivers: net: xgene: fix statistics counters race condition" from May 13, 2016, leads to the following static checker warning: drivers/net/ethernet/apm/xgene/xgene_enet_main.c:487 xgene_enet_rx_frame() warn: should this be a bitwise op? drivers/net/ethernet/apm/xgene/xgene_enet_main.c 472 u8 status; 473 int ret = 0; 474 475 ndev = rx_ring->ndev; 476 pdata = netdev_priv(ndev); 477 dev = ndev_to_dev(rx_ring->ndev); 478 buf_pool = rx_ring->buf_pool; 479 480 dma_unmap_single(dev, GET_VAL(DATAADDR, le64_to_cpu(raw_desc->m1)), 481 XGENE_ENET_MAX_MTU, DMA_FROM_DEVICE); 482 skb_index = GET_VAL(USERINFO, le64_to_cpu(raw_desc->m0)); 483 skb = buf_pool->rx_skb[skb_index]; 484 485 /* checking for error */ 486 status = (GET_VAL(ELERR, le64_to_cpu(raw_desc->m0)) << LERR_LEN) || 487GET_VAL(LERR, le64_to_cpu(raw_desc->m0)); 488 if (unlikely(status > 2)) { This code doesn't make sense and I don't know what is intended. LERR_LEN is 3, but why are we shifting when we just care about true/false? In the original code status could be > 2 but now it's either 0 or 1. 489 dev_kfree_skb_any(skb); 490 xgene_enet_parse_error(rx_ring, netdev_priv(rx_ring->ndev), 491 status); 492 ret = -EIO; 493 goto out; 494 } regards, dan carpenter
[patch] qed: Remove a stray tab
This line was intended more than it should be. Signed-off-by: Dan Carpenter diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c b/drivers/net/ethernet/qlogic/qed/qed_main.c index 6ffc21d..05fcb25 100644 --- a/drivers/net/ethernet/qlogic/qed/qed_main.c +++ b/drivers/net/ethernet/qlogic/qed/qed_main.c @@ -177,7 +177,7 @@ static int qed_init_pci(struct qed_dev *cdev, } if (IS_PF(cdev)) { - cdev->db_phys_addr = pci_resource_start(cdev->pdev, 2); + cdev->db_phys_addr = pci_resource_start(cdev->pdev, 2); cdev->db_size = pci_resource_len(cdev->pdev, 2); cdev->doorbells = ioremap_wc(cdev->db_phys_addr, cdev->db_size); if (!cdev->doorbells) {
[patch v2] qed: Remove a stray tab
This line was indented more than it should be. Signed-off-by: Dan Carpenter Acked-by: Yuval Mintz --- v2: fix a typo diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c b/drivers/net/ethernet/qlogic/qed/qed_main.c index 6ffc21d..05fcb25 100644 --- a/drivers/net/ethernet/qlogic/qed/qed_main.c +++ b/drivers/net/ethernet/qlogic/qed/qed_main.c @@ -177,7 +177,7 @@ static int qed_init_pci(struct qed_dev *cdev, } if (IS_PF(cdev)) { - cdev->db_phys_addr = pci_resource_start(cdev->pdev, 2); + cdev->db_phys_addr = pci_resource_start(cdev->pdev, 2); cdev->db_size = pci_resource_len(cdev->pdev, 2); cdev->doorbells = ioremap_wc(cdev->db_phys_addr, cdev->db_size); if (!cdev->doorbells) {
[patch] qed: signedness bug in qed_dcbx_process_tlv()
"priority" needs to be signed for the error handling to work. Fixes: 39651abd2814 ('qed: add support for dcbx.') Signed-off-by: Dan Carpenter --- Applies to the main net tree. diff --git a/drivers/net/ethernet/qlogic/qed/qed_dcbx.c b/drivers/net/ethernet/qlogic/qed/qed_dcbx.c index cbf58e1..a06d19a 100644 --- a/drivers/net/ethernet/qlogic/qed/qed_dcbx.c +++ b/drivers/net/ethernet/qlogic/qed/qed_dcbx.c @@ -192,9 +192,10 @@ qed_dcbx_process_tlv(struct qed_hwfn *p_hwfn, struct dcbx_app_priority_entry *p_tbl, u32 pri_tc_tbl, int count, bool dcbx_enabled) { - u8 tc, priority, priority_map; + u8 tc, priority_map; enum dcbx_protocol_type type; u16 protocol_id; + int priority; bool enable; int i;
[patch] ptp: oops in ptp_ioctl()
If we pass ERR_PTR(-EFAULT) to kfree() then it's going to oops. Fixes: 2ece068e1b1d ('ptp: use memdup_user().') Signed-off-by: Dan Carpenter diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c index 0b1ac6b..d637c93 100644 --- a/drivers/ptp/ptp_chardev.c +++ b/drivers/ptp/ptp_chardev.c @@ -211,6 +211,7 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg) sysoff = memdup_user((void __user *)arg, sizeof(*sysoff)); if (IS_ERR(sysoff)) { err = PTR_ERR(sysoff); + sysoff = NULL; break; } if (sysoff->n_samples > PTP_MAX_SAMPLES) {
[patch] atm: firestream: add more reserved strings
This bug was there when the driver was first added in back in year 2000. It causes a Smatch warning: drivers/atm/firestream.c:849 process_incoming() error: buffer overflow 'res_strings' 60 <= 63 There are supposed to be 64 entries in this array and the missing strings are clearly in the 30 40 range. I added them as reserved 37 to reserved 40. It's possible that strings are really supposed to be added in the middle instead of at the end, but this approach is safe, in that it fixes the bug and doesn't break anything that wasn't already broken. Signed-off-by: Dan Carpenter diff --git a/drivers/atm/firestream.c b/drivers/atm/firestream.c index a969a7e..4777064 100644 --- a/drivers/atm/firestream.c +++ b/drivers/atm/firestream.c @@ -181,13 +181,17 @@ static char *res_strings[] = { "reserved 27", "reserved 28", "reserved 29", - "reserved 30", + "reserved 30", /* FIXME: The strings between 30-40 might be wrong. */ "reassembly abort: no buffers", "receive buffer overflow", "change in GFC", "receive buffer full", "low priority discard - no receive descriptor", "low priority discard - missing end of packet", + "reserved 37", + "reserved 38", + "reserved 39", + "reseverd 40", "reserved 41", "reserved 42", "reserved 43",
[patch] atm: iphase: off by one in rx_pkt()
The iadev->rx_open[] array holds "iadev->num_vc" pointers (this code assumes that pointers are 32 bits). So the > here should be >= or else we could end up reading a garbage pointer from one element beyond the end of the array. Signed-off-by: Dan Carpenter diff --git a/drivers/atm/iphase.c b/drivers/atm/iphase.c index 7d00f29..f86e318 100644 --- a/drivers/atm/iphase.c +++ b/drivers/atm/iphase.c @@ -1128,7 +1128,7 @@ static int rx_pkt(struct atm_dev *dev) /* make the ptr point to the corresponding buffer desc entry */ buf_desc_ptr += desc; if (!desc || (desc > iadev->num_rx_desc) || - ((buf_desc_ptr->vc_index & 0x) > iadev->num_vc)) { + ((buf_desc_ptr->vc_index & 0x) >= iadev->num_vc)) { free_desc(dev, desc); IF_ERR(printk("IA: bad descriptor desc = %d \n", desc);) return -1;
[patch] rocker: fix rocker_world_port_obj_vlan_add()
We were changing return values and accidentally made rocker_world_port_obj_vlan_add() into a no-op. Fixes: fccd84d44912 ('rocker: return -EOPNOTSUPP for undefined world ops') Signed-off-by: Dan Carpenter diff --git a/drivers/net/ethernet/rocker/rocker_main.c b/drivers/net/ethernet/rocker/rocker_main.c index acafbf8..28b775e 100644 --- a/drivers/net/ethernet/rocker/rocker_main.c +++ b/drivers/net/ethernet/rocker/rocker_main.c @@ -1598,7 +1598,6 @@ rocker_world_port_obj_vlan_add(struct rocker_port *rocker_port, if (!wops->port_obj_vlan_add) return -EOPNOTSUPP; - return 0; return wops->port_obj_vlan_add(rocker_port, vlan, trans); }
[patch 1/2 net-next] mediatek: checking for IS_ERR() instead of NULL
of_phy_connect() returns NULL on error, it never returns error pointers. Fixes: 656e705243fd ('net-next: mediatek: add support for MT7623 ethernet') Signed-off-by: Dan Carpenter diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c index ba3afa5..9759fe5 100644 --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c @@ -186,9 +186,9 @@ static int mtk_phy_connect_node(struct mtk_eth *eth, struct mtk_mac *mac, phydev = of_phy_connect(eth->netdev[mac->id], phy_node, mtk_phy_link_adjust, 0, phy_mode); - if (IS_ERR(phydev)) { + if (!phydev) { dev_err(eth->dev, "could not connect to PHY\n"); - return PTR_ERR(phydev); + return -ENODEV; } dev_info(eth->dev,
[patch 2/2 net-next] mediatek: unlock on error in mtk_tx_map()
There was a missing unlock on the error path. Fixes: 656e705243fd ('net-next: mediatek: add support for MT7623 ethernet') Signed-off-by: Dan Carpenter diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c index 9759fe5..c2c2e206 100644 --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c @@ -661,6 +661,8 @@ err_dma: itxd = mtk_qdma_phys_to_virt(ring, itxd->txd2); } while (itxd != txd); + spin_unlock_irqrestore(ð->page_lock, flags); + return -ENOMEM; }
[patch] ethernet: micrel: fix some error codes
There were two issues here: 1) dma_mapping_error() return true/false but we want to return -ENOMEM 2) If dmaengine_prep_slave_sg() failed then "err" wasn't set but presumably that should be -ENOMEM as well. I changed the success path to "return 0;" instead of "return ret;" for clarity. Fixes: 94fe8c683cea ('ks8842: Support DMA when accessed via timberdale') Signed-off-by: Dan Carpenter diff --git a/drivers/net/ethernet/micrel/ks8842.c b/drivers/net/ethernet/micrel/ks8842.c index 09d2e16..cb0102d 100644 --- a/drivers/net/ethernet/micrel/ks8842.c +++ b/drivers/net/ethernet/micrel/ks8842.c @@ -561,8 +561,8 @@ static int __ks8842_start_new_rx_dma(struct net_device *netdev) sg_init_table(sg, 1); sg_dma_address(sg) = dma_map_single(adapter->dev, ctl->skb->data, DMA_BUFFER_SIZE, DMA_FROM_DEVICE); - err = dma_mapping_error(adapter->dev, sg_dma_address(sg)); - if (unlikely(err)) { + if (dma_mapping_error(adapter->dev, sg_dma_address(sg))) { + err = -ENOMEM; sg_dma_address(sg) = 0; goto out; } @@ -572,8 +572,10 @@ static int __ks8842_start_new_rx_dma(struct net_device *netdev) ctl->adesc = dmaengine_prep_slave_sg(ctl->chan, sg, 1, DMA_DEV_TO_MEM, DMA_PREP_INTERRUPT); - if (!ctl->adesc) + if (!ctl->adesc) { + err = -ENOMEM; goto out; + } ctl->adesc->callback_param = netdev; ctl->adesc->callback = ks8842_dma_rx_cb; @@ -584,7 +586,7 @@ static int __ks8842_start_new_rx_dma(struct net_device *netdev) goto out; } - return err; + return 0; out: if (sg_dma_address(sg)) dma_unmap_single(adapter->dev, sg_dma_address(sg),
[patch] mdio-sun4i: oops in error handling in probe
We could end up dereferencing an error pointer when we call regulator_disable(). Fixes: 4bdcb1dd9feb ('net: Add MDIO bus driver for the Allwinner EMAC') Signed-off-by: Dan Carpenter diff --git a/drivers/net/phy/mdio-sun4i.c b/drivers/net/phy/mdio-sun4i.c index f70522c..1352965 100644 --- a/drivers/net/phy/mdio-sun4i.c +++ b/drivers/net/phy/mdio-sun4i.c @@ -122,6 +122,7 @@ static int sun4i_mdio_probe(struct platform_device *pdev) return -EPROBE_DEFER; dev_info(&pdev->dev, "no regulator found\n"); + data->regulator = NULL; } else { ret = regulator_enable(data->regulator); if (ret) @@ -137,7 +138,8 @@ static int sun4i_mdio_probe(struct platform_device *pdev) return 0; err_out_disable_regulator: - regulator_disable(data->regulator); + if (data->regulator) + regulator_disable(data->regulator); err_out_free_mdiobus: mdiobus_free(bus); return ret;
[patch] rocker: fix an error code
We intended to return PTR_ERR() here instead of 1. Fixes: 1f9993f6825f ('rocker: fix a neigh entry leak issue') Signed-off-by: Dan Carpenter --- We recently moved rocker files around so this only applies to -next. Probably returning the wrong error code is harmless. diff --git a/drivers/net/ethernet/rocker/rocker_ofdpa.c b/drivers/net/ethernet/rocker/rocker_ofdpa.c index 099008a..07218c3 100644 --- a/drivers/net/ethernet/rocker/rocker_ofdpa.c +++ b/drivers/net/ethernet/rocker/rocker_ofdpa.c @@ -1449,7 +1449,7 @@ static int ofdpa_port_ipv4_resolve(struct ofdpa_port *ofdpa_port, if (!n) { n = neigh_create(&arp_tbl, &ip_addr, dev); if (IS_ERR(n)) - return IS_ERR(n); + return PTR_ERR(n); } /* If the neigh is already resolved, then go ahead and
[patch] net: moxa: fix an error code
We accidentally return IS_ERR(priv->base) which is 1 instead of PTR_ERR(priv->base) which is the error code. Fixes: 6c821bd9edc9 ('net: Add MOXA ART SoCs ethernet driver') Signed-off-by: Dan Carpenter diff --git a/drivers/net/ethernet/moxa/moxart_ether.c b/drivers/net/ethernet/moxa/moxart_ether.c index 00cfd95..3e67f45 100644 --- a/drivers/net/ethernet/moxa/moxart_ether.c +++ b/drivers/net/ethernet/moxa/moxart_ether.c @@ -474,9 +474,9 @@ static int moxart_mac_probe(struct platform_device *pdev) res = platform_get_resource(pdev, IORESOURCE_MEM, 0); ndev->base_addr = res->start; priv->base = devm_ioremap_resource(p_dev, res); - ret = IS_ERR(priv->base); - if (ret) { + if (IS_ERR(priv->base)) { dev_err(p_dev, "devm_ioremap_resource failed\n"); + ret = PTR_ERR(priv->base); goto init_fail; }
Re: [patch] net: moxa: fix an error code
On Wed, Mar 02, 2016 at 11:52:29AM +0100, Arnd Bergmann wrote: > Did you find more of these? > > it doesn't matter much either way, but if you do multiple such patches, One or two. I already sent the fixes. I think it was applied. > I'd suggest using a single PTR_ERR_OR_ZERO() instead of IS_ERR()+PTR_ERR(). > > I have found a couple of drivers in which that leads to better object > code, and avoids a warning about a possibly uninitialized variable > when the function gets inlined into another one (which won't happen > for this driver). Huh? I sent one where I could have done that but I deliberately didn't because I wanted the uninitialized warning if I made a mistake. It sounds like you're working around a GCC bug... regards, dan carpenter
Re: [patch] net: moxa: fix an error code
On Wed, Mar 02, 2016 at 12:36:05PM +0100, Arnd Bergmann wrote: > The uninitialized warning here is about a type mismatch preventing > gcc from noticing that two conditions are the same, I'm not sure > if this is a bug in gcc, or required by the C standard. I wouldn't call it a bug, because everyone has to make trade offs between how fast the program runs and how accurate it is. And trade offs between how ambitious your warnings are vs how many false positives you can tolerate. Anyway, I feel like we should just work around GCC on a case by case basis instead of always using PTR_ERR_OR_ZERO(). The next version of GCC will fix some false positives and introduce new ones... Next time using PTR_ERR_OR_ZERO() could cause warnings instead of fixing them. Smatch works in a different way and it parse the code correctly. But Smatch is slow and sometimes runs out of memory and gives up trying to parse large functions. Smatch sees the two returns and tries to figure out the implications of each (uninitialized vs initialized). If you change the code to: error = PTR_ERR_OR_ZERO(hash); if (!error) *leaf_out = be64_to_cpu(*(hash + index)); return error; then Smatch still breaks that up into two separate returns which imply initialized vs uninitialized. regards, dan carpenter
[patch] libertas: fix an error code in probe
We accidentally return success instead of a negative error code. Signed-off-by: Dan Carpenter diff --git a/drivers/net/wireless/marvell/libertas/main.c b/drivers/net/wireless/marvell/libertas/main.c index b35b8bc..8541cbe 100644 --- a/drivers/net/wireless/marvell/libertas/main.c +++ b/drivers/net/wireless/marvell/libertas/main.c @@ -1118,7 +1118,8 @@ int lbs_start_card(struct lbs_private *priv) else pr_info("%s: mesh disabled\n", dev->name); - if (lbs_cfg_register(priv)) { + ret = lbs_cfg_register(priv); + if (ret) { pr_err("cannot register device\n"); goto done; }
Re: Inconsistent use of size argument in kzalloc and memcpy in 'drivers/net/ethernet/toshiba/ps3_gelic_wireless.c'
On Mon, Apr 11, 2016 at 12:00:04PM +0200, Christophe JAILLET wrote: > Hi, > > while looking at potential clean-up, I ended on the following code > which looks spurious to me. > > We allocate 'be16_to_cpu(scan_info->size)' bytes, but then copy > 'scan_info->size'. > This is not consistent. > Good catch. be16_to_cpu(scan_info->size) is correct. It's surprising that this bug wasn't caught in testing... regards, dan carpenter
[patch -next] udp: fix if statement in SIOCINQ ioctl
We deleted a line of code and accidentally made the "return put_user()" part of the if statement when it's supposed to be unconditional. Fixes: 9f9a45beaa96 ('udp: do not expect udp headers on ioctl SIOCINQ') Signed-off-by: Dan Carpenter diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c index f186313..37e09c3 100644 --- a/net/ipv4/udp.c +++ b/net/ipv4/udp.c @@ -1276,12 +1276,6 @@ int udp_ioctl(struct sock *sk, int cmd, unsigned long arg) { unsigned int amount = first_packet_length(sk); - if (amount) - /* -* We will only return the amount -* of this packet since that is all -* that will be read. -*/ return put_user(amount, (int __user *)arg); }
[patch -next] geneve: testing the wrong variable in geneve6_build_skb()
We intended to test "err" and not "skb". Fixes: aed069df099c ('ip_tunnel_core: iptunnel_handle_offloads returns int and doesn't free skb') Signed-off-by: Dan Carpenter diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c index efbc7ce..512dbe0 100644 --- a/drivers/net/geneve.c +++ b/drivers/net/geneve.c @@ -733,7 +733,7 @@ static int geneve6_build_skb(struct dst_entry *dst, struct sk_buff *skb, goto free_dst; err = udp_tunnel_handle_offloads(skb, udp_sum); - if (IS_ERR(skb)) + if (err) goto free_dst; gnvh = (struct genevehdr *)__skb_push(skb, sizeof(*gnvh) + opt_len);
[patch] tipc: remove an unnecessary NULL check
This is never called with a NULL "buf" and anyway, we dereference 's' on the lines before so it would Oops before we reach the check. Signed-off-by: Dan Carpenter diff --git a/net/tipc/subscr.c b/net/tipc/subscr.c index 79de588..0dd0224 100644 --- a/net/tipc/subscr.c +++ b/net/tipc/subscr.c @@ -326,8 +326,7 @@ static void tipc_subscrb_rcv_cb(struct net *net, int conid, return tipc_subscrp_cancel(s, subscriber); } - if (s) - tipc_subscrp_subscribe(net, s, subscriber, swap); + tipc_subscrp_subscribe(net, s, subscriber, swap); } /* Handle one request to establish a new subscriber */
re: net: mvpp2: fix missing DMA region unmap in egress processing
Hello Marcin Wojtas, The patch e864b4c7b184: "net: mvpp2: fix missing DMA region unmap in egress processing" from Dec 3, 2015, leads to the following static checker warning: drivers/net/ethernet/marvell/mvpp2.c:4414 mvpp2_txq_bufs_free() warn: variable dereferenced before check 'skb' (see line 4412) drivers/net/ethernet/marvell/mvpp2.c 4399 static void mvpp2_txq_bufs_free(struct mvpp2_port *port, 4400 struct mvpp2_tx_queue *txq, 4401 struct mvpp2_txq_pcpu *txq_pcpu, int num) 4402 { 4403 int i; 4404 4405 for (i = 0; i < num; i++) { 4406 dma_addr_t buf_phys_addr = 4407 txq_pcpu->tx_buffs[txq_pcpu->txq_get_index]; 4408 struct sk_buff *skb = txq_pcpu->tx_skb[txq_pcpu->txq_get_index]; 4409 4410 mvpp2_txq_inc_get(txq_pcpu); 4411 4412 dma_unmap_single(port->dev->dev.parent, buf_phys_addr, 4413 skb_headlen(skb), DMA_TO_DEVICE); Dereference inside function. 4414 if (!skb) ^ Check too late. There is no way that patch can be fix a bug, the best case is that it does nothing or maybe it adds an oops... 4415 continue; 4416 dev_kfree_skb_any(skb); 4417 } 4418 } regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch -next] VSOCK: signedness bug in virtio_transport_dgram_enqueue()
"written" has to be signed for the error handling to work. trans->ops->send_pkt() returns an int so that's fine. Fixes: 80a19e338d45 ('VSOCK: Introduce virtio-vsock-common.ko') Signed-off-by: Dan Carpenter diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c index 28f790d..d9a2325 100644 --- a/net/vmw_vsock/virtio_transport_common.c +++ b/net/vmw_vsock/virtio_transport_common.c @@ -815,7 +815,8 @@ virtio_transport_dgram_enqueue(struct vsock_sock *vsk, .type = VIRTIO_VSOCK_TYPE_DGRAM, .msg = msg, }; - size_t total_written = 0, pkt_off = 0, written; + size_t total_written = 0, pkt_off = 0; + int written; u16 dgram_id; /* The max size of a single dgram we support is 64KB */ @@ -845,7 +846,7 @@ virtio_transport_dgram_enqueue(struct vsock_sock *vsk, } total_written += written; pkt_off += written; - pr_debug("%s:id=%d, dgram_len=%zu, off=%zu, total_written=%zu, written=%zu\n", + pr_debug("%s:id=%d, dgram_len=%zu, off=%zu, total_written=%zu, written=%d\n", __func__, dgram_id, dgram_len, pkt_off, total_written, written); } -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch -next] mlxsw: spectrum: fix some error handling
The "err = " assignment is missing here. Fixes: 0d65fc13042f ('mlxsw: spectrum: Implement LAG port join/leave') Signed-off-by: Dan Carpenter diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c index 3ec07b9..322ed54 100644 --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c @@ -2091,7 +2091,7 @@ static int mlxsw_sp_port_lag_leave(struct mlxsw_sp_port *mlxsw_sp_port, err = mlxsw_sp_lag_col_port_disable(mlxsw_sp_port, lag_id); if (err) return err; - mlxsw_sp_lag_col_port_remove(mlxsw_sp_port, lag_id); + err = mlxsw_sp_lag_col_port_remove(mlxsw_sp_port, lag_id); if (err) return err; -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch -next] mlxsw: core: remove an unneeded condition
We already know "err" is zero so there is no need to check. Signed-off-by: Dan Carpenter diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_hwmon.c b/drivers/net/ethernet/mellanox/mlxsw/core_hwmon.c index 4dad146..913106d 100644 --- a/drivers/net/ethernet/mellanox/mlxsw/core_hwmon.c +++ b/drivers/net/ethernet/mellanox/mlxsw/core_hwmon.c @@ -169,7 +169,7 @@ static ssize_t mlxsw_hwmon_pwm_store(struct device *dev, dev_err(mlxsw_hwmon->bus_info->dev, "Failed to write PWM\n"); return err; } - return err ? err : len; + return len; } enum mlxsw_hwmon_attr_type { -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
re: [PATCH] chelsio: add support for other 10G boards
Hello Stephen Hemminger, The patch f1d3d38af757: "[PATCH] chelsio: add support for other 10G boards" from Dec 1, 2006, leads to the following static checker warning: drivers/net/ethernet/chelsio/cxgb/subr.c:630 t1_link_start() warn: was shift intended here '(mac->adapter->params.nports < 2)' drivers/net/ethernet/chelsio/cxgb/subr.c 623 int t1_link_start(struct cphy *phy, struct cmac *mac, struct link_config *lc) 624 { 625 unsigned int fc = lc->requested_fc & (PAUSE_RX | PAUSE_TX); 626 627 if (lc->supported & SUPPORTED_Autoneg) { 628 lc->advertising &= ~(ADVERTISED_ASYM_PAUSE | ADVERTISED_PAUSE); 629 if (fc) { 630 if (fc == ((PAUSE_RX | PAUSE_TX) & 631 (mac->adapter->params.nports < 2))) This condition is never weird. PAUSE_RX is 1. PAUSE_TX is 2. The nports < 2 condition is either 0 or 1. We know fc is in 1-3 range. We could re-write it as: if (fc == 1 && mac->adapter->params.nports < 2) The static checker is suggesting that we could do nports << 2 but then the condition would never be true so that can't be right. 632 lc->advertising |= ADVERTISED_PAUSE; 633 else { 634 lc->advertising |= ADVERTISED_ASYM_PAUSE; 635 if (fc == PAUSE_RX) 636 lc->advertising |= ADVERTISED_PAUSE; 637 } 638 } 639 phy->ops->advertise(phy, lc->advertising); regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] mISDN: fix a loop count
There are two issue here. 1) cnt starts as maxloop + 1 so all these loops iterate one more time than intended. 2) At the end of the loop we test for "if (maxloop && !cnt)" but for the first two loops, we end with cnt equal to -1. Changing this to a pre-op means we end with cnt set to 0. Fixes: cae86d4a4e56 ('mISDN: Add driver for Infineon ISDN chipset family') Signed-off-by: Dan Carpenter diff --git a/drivers/isdn/hardware/mISDN/mISDNipac.c b/drivers/isdn/hardware/mISDN/mISDNipac.c index a77eea5..cb428b9 100644 --- a/drivers/isdn/hardware/mISDN/mISDNipac.c +++ b/drivers/isdn/hardware/mISDN/mISDNipac.c @@ -1170,7 +1170,7 @@ mISDNipac_irq(struct ipac_hw *ipac, int maxloop) if (ipac->type & IPAC_TYPE_IPACX) { ista = ReadIPAC(ipac, ISACX_ISTA); - while (ista && cnt--) { + while (ista && --cnt) { pr_debug("%s: ISTA %02x\n", ipac->name, ista); if (ista & IPACX__ICA) ipac_irq(&ipac->hscx[0], ista); @@ -1182,7 +1182,7 @@ mISDNipac_irq(struct ipac_hw *ipac, int maxloop) } } else if (ipac->type & IPAC_TYPE_IPAC) { ista = ReadIPAC(ipac, IPAC_ISTA); - while (ista && cnt--) { + while (ista && --cnt) { pr_debug("%s: ISTA %02x\n", ipac->name, ista); if (ista & (IPAC__ICD | IPAC__EXD)) { istad = ReadISAC(isac, ISAC_ISTA); @@ -1200,7 +1200,7 @@ mISDNipac_irq(struct ipac_hw *ipac, int maxloop) ista = ReadIPAC(ipac, IPAC_ISTA); } } else if (ipac->type & IPAC_TYPE_HSCX) { - while (cnt) { + while (--cnt) { ista = ReadIPAC(ipac, IPAC_ISTAB + ipac->hscx[1].off); pr_debug("%s: B2 ISTA %02x\n", ipac->name, ista); if (ista) @@ -1211,7 +1211,6 @@ mISDNipac_irq(struct ipac_hw *ipac, int maxloop) mISDNisac_irq(isac, istad); if (0 == (ista | istad)) break; - cnt--; } } if (cnt > maxloop) /* only for ISAC/HSCX without PCI IRQ test */ -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch -mainline] amd-xgbe: fix a couple timeout loops
At the end of the loop we test "if (!count)" but because "count--" is a post-op then the loop will end with count set to -1. I have fixed this by changing it to --count. Fixes: c5aa9e3b8156 ('amd-xgbe: Initial AMD 10GbE platform driver') Signed-off-by: Dan Carpenter diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-dev.c b/drivers/net/ethernet/amd/xgbe/xgbe-dev.c index 970781a..f6a7161 100644 --- a/drivers/net/ethernet/amd/xgbe/xgbe-dev.c +++ b/drivers/net/ethernet/amd/xgbe/xgbe-dev.c @@ -1849,7 +1849,7 @@ static int xgbe_exit(struct xgbe_prv_data *pdata) usleep_range(10, 15); /* Poll Until Poll Condition */ - while (count-- && XGMAC_IOREAD_BITS(pdata, DMA_MR, SWR)) + while (--count && XGMAC_IOREAD_BITS(pdata, DMA_MR, SWR)) usleep_range(500, 600); if (!count) @@ -1873,7 +1873,7 @@ static int xgbe_flush_tx_queues(struct xgbe_prv_data *pdata) /* Poll Until Poll Condition */ for (i = 0; i < pdata->tx_q_count; i++) { count = 2000; - while (count-- && XGMAC_MTL_IOREAD_BITS(pdata, i, + while (--count && XGMAC_MTL_IOREAD_BITS(pdata, i, MTL_Q_TQOMR, FTQ)) usleep_range(500, 600); -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] qlcnic: fix a timeout loop
The problem here is that at the end of the loop we test for if idc->vnic_wait_limit is zero, but since idc->vnic_wait_limit-- is a post-op, it actually ends up set to (u8)-1. I have fixed this by changing it to a pre-op. I had to change the starting value from "QLCNIC_DEV_NPAR_OPER_TIMEO" (30) to "QLCNIC_DEV_NPAR_OPER_TIMEO + 1" so that we still loop the same number of times as before. Fixes: 486a5bc77a4a ('qlcnic: Add support for 83xx suspend and resume.') Signed-off-by: Dan Carpenter diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_vnic.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_vnic.c index be7d7a6..9919245 100644 --- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_vnic.c +++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_vnic.c @@ -234,7 +234,7 @@ int qlcnic_83xx_config_vnic_opmode(struct qlcnic_adapter *adapter) } ahw->idc.vnic_state = QLCNIC_DEV_NPAR_NON_OPER; - ahw->idc.vnic_wait_limit = QLCNIC_DEV_NPAR_OPER_TIMEO; + ahw->idc.vnic_wait_limit = QLCNIC_DEV_NPAR_OPER_TIMEO + 1; return 0; } @@ -246,7 +246,7 @@ int qlcnic_83xx_check_vnic_state(struct qlcnic_adapter *adapter) u32 state; state = QLCRDX(ahw, QLC_83XX_VNIC_STATE); - while (state != QLCNIC_DEV_NPAR_OPER && idc->vnic_wait_limit--) { + while (state != QLCNIC_DEV_NPAR_OPER && --idc->vnic_wait_limit) { msleep(1000); state = QLCRDX(ahw, QLC_83XX_VNIC_STATE); } -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch -mainline] qlge: fix a timeout loop in ql_change_rx_buffers()
The problem here is that after the loop we test for "if (!i) " but because "i--" is a post-op we exit with i set to -1. I have fixed this by changing it to a pre-op instead. I had to change the starting value from 3 to 4 so that we still iterate 3 times. Signed-off-by: Dan Carpenter diff --git a/drivers/net/ethernet/qlogic/qlge/qlge_main.c b/drivers/net/ethernet/qlogic/qlge/qlge_main.c index 02b7115..9979764 100644 --- a/drivers/net/ethernet/qlogic/qlge/qlge_main.c +++ b/drivers/net/ethernet/qlogic/qlge/qlge_main.c @@ -4211,8 +4211,9 @@ static int ql_change_rx_buffers(struct ql_adapter *qdev) /* Wait for an outstanding reset to complete. */ if (!test_bit(QL_ADAPTER_UP, &qdev->flags)) { - int i = 3; - while (i-- && !test_bit(QL_ADAPTER_UP, &qdev->flags)) { + int i = 4; + + while (--i && !test_bit(QL_ADAPTER_UP, &qdev->flags)) { netif_err(qdev, ifup, qdev->ndev, "Waiting for adapter UP...\n"); ssleep(1); -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch -mainline] sfc: fix a timeout loop
We test for if "tries" is zero at the end but "tries--" is a post-op so it will end with "tries" set to -1. I have changed it to a pre-op instead. Signed-off-by: Dan Carpenter diff --git a/drivers/net/ethernet/sfc/txc43128_phy.c b/drivers/net/ethernet/sfc/txc43128_phy.c index 3d5ee32..194f67d 100644 --- a/drivers/net/ethernet/sfc/txc43128_phy.c +++ b/drivers/net/ethernet/sfc/txc43128_phy.c @@ -418,7 +418,7 @@ static void txc_reset_logic_mmd(struct efx_nic *efx, int mmd) val |= (1 << TXC_GLCMD_LMTSWRST_LBN); efx_mdio_write(efx, mmd, TXC_GLRGS_GLCMD, val); - while (tries--) { + while (--tries) { val = efx_mdio_read(efx, mmd, TXC_GLRGS_GLCMD); if (!(val & (1 << TXC_GLCMD_LMTSWRST_LBN))) break; -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch v2] qlcnic: fix a timeout loop
The problem here is that at the end of the loop we test for if idc->vnic_wait_limit is zero, but since idc->vnic_wait_limit-- is a post-op, it actually ends up set to (u8)-1. I have fixed this by moving the decrement inside the loop. Fixes: 486a5bc77a4a ('qlcnic: Add support for 83xx suspend and resume.') Signed-off-by: Dan Carpenter --- v2: different color on the bikeshed diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_vnic.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_vnic.c index be7d7a6..b1a452f 100644 --- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_vnic.c +++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_vnic.c @@ -246,7 +246,8 @@ int qlcnic_83xx_check_vnic_state(struct qlcnic_adapter *adapter) u32 state; state = QLCRDX(ahw, QLC_83XX_VNIC_STATE); - while (state != QLCNIC_DEV_NPAR_OPER && idc->vnic_wait_limit--) { + while (state != QLCNIC_DEV_NPAR_OPER && idc->vnic_wait_limit) { + idc->vnic_wait_limit--; msleep(1000); state = QLCRDX(ahw, QLC_83XX_VNIC_STATE); } -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch v2] qlcnic: fix a timeout loop
On Tue, Dec 15, 2015 at 01:13:41PM -0500, David Miller wrote: > From: Dan Carpenter > Date: Tue, 15 Dec 2015 16:56:16 +0300 > > > The problem here is that at the end of the loop we test for if > > idc->vnic_wait_limit is zero, but since idc->vnic_wait_limit-- is a > > post-op, it actually ends up set to (u8)-1. I have fixed this by > > moving the decrement inside the loop. > > > > Fixes: 486a5bc77a4a ('qlcnic: Add support for 83xx suspend and resume.') > > Signed-off-by: Dan Carpenter > > Applied. Ugh... I appologize, I just noticed a mistake in this one. I have to do the decrement at the end of the loop and not the start of the loop. I will send a fix for that on top of this fix. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch v2] qlcnic: fix a timeout loop
On Tue, Dec 15, 2015 at 11:13:50PM +0300, Dan Carpenter wrote: > On Tue, Dec 15, 2015 at 01:13:41PM -0500, David Miller wrote: > > From: Dan Carpenter > > Date: Tue, 15 Dec 2015 16:56:16 +0300 > > > > > The problem here is that at the end of the loop we test for if > > > idc->vnic_wait_limit is zero, but since idc->vnic_wait_limit-- is a > > > post-op, it actually ends up set to (u8)-1. I have fixed this by > > > moving the decrement inside the loop. > > > > > > Fixes: 486a5bc77a4a ('qlcnic: Add support for 83xx suspend and resume.') > > > Signed-off-by: Dan Carpenter > > > > Applied. > > Ugh... I appologize, I just noticed a mistake in this one. I have to > do the decrement at the end of the loop and not the start of the loop. > Nope. I'm wrong again. My original patch was the correct way to do this. Otherwise you can end up with state == QLCNIC_DEV_NPAR_OPER and ->vnic_wait_limit set to zero. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch -next] tipc: potential shift wrapping bug in map_set()
"up_map" is a u64 type but we're not using the high 32 bits. Fixes: 35c55c9877f8 ('tipc: add neighbor monitoring framework') Signed-off-by: Dan Carpenter diff --git a/net/tipc/monitor.c b/net/tipc/monitor.c index 87d4efe..0d489e8 100644 --- a/net/tipc/monitor.c +++ b/net/tipc/monitor.c @@ -122,8 +122,8 @@ static int dom_size(int peers) static void map_set(u64 *up_map, int i, unsigned int v) { - *up_map &= ~(1 << i); - *up_map |= (v << i); + *up_map &= ~(1ULL << i); + *up_map |= ((u64)v << i); } static int map_get(u64 up_map, int i)
[patch net-next] rxrpc: checking for IS_ERR() instead of NULL
rxrpc_lookup_peer_rcu() returns NULL on error, it never returns error pointers. Fixes: be6e6707f6ee ('rxrpc: Rework peer object handling to use hash table and RCU') Signed-off-by: Dan Carpenter diff --git a/net/rxrpc/input.c b/net/rxrpc/input.c index 47fb167..e11e4d7 100644 --- a/net/rxrpc/input.c +++ b/net/rxrpc/input.c @@ -639,7 +639,7 @@ static struct rxrpc_connection *rxrpc_conn_from_local(struct rxrpc_local *local, rxrpc_get_addr_from_skb(local, skb, &srx); rcu_read_lock(); peer = rxrpc_lookup_peer_rcu(local, &srx); - if (IS_ERR(peer)) + if (!peer) goto cant_find_peer; trans = rxrpc_find_transport(local, peer);
[patch 1/3 -next] liquidio: a couple indenting tweaks
Remove a stray space character and fix an if statement that wasn't indented. Signed-off-by: Dan Carpenter --- Speaking of style issues, this whole file follows return 1 on error and 0 on success. It was confusing for me. It's not too late to change it to use correct error codes. ;) diff --git a/drivers/net/ethernet/cavium/liquidio/lio_main.c b/drivers/net/ethernet/cavium/liquidio/lio_main.c index d0ab97c..715ddfa 100644 --- a/drivers/net/ethernet/cavium/liquidio/lio_main.c +++ b/drivers/net/ethernet/cavium/liquidio/lio_main.c @@ -365,7 +365,7 @@ static int wait_for_pending_requests(struct octeon_device *oct) [OCTEON_ORDERED_SC_LIST].pending_req_count); if (pcount) schedule_timeout_uninterruptible(HZ / 10); -else + else break; } @@ -3351,8 +3351,8 @@ static int setup_nic_devices(struct octeon_device *octeon_dev) liquidio_set_ethtool_ops(netdev); if (netdev->features & NETIF_F_LRO) - liquidio_set_feature(netdev, OCTNET_CMD_LRO_ENABLE, -OCTNIC_LROIPV4 | OCTNIC_LROIPV6); + liquidio_set_feature(netdev, OCTNET_CMD_LRO_ENABLE, +OCTNIC_LROIPV4 | OCTNIC_LROIPV6); if ((debug != -1) && (debug & NETIF_MSG_HW)) liquidio_set_feature(netdev, OCTNET_CMD_VERBOSE_ENABLE,
[patch 2/3] liquidio: remove an unused variable
We don't use "i" in this function. Signed-off-by: Dan Carpenter diff --git a/drivers/net/ethernet/cavium/liquidio/lio_main.c b/drivers/net/ethernet/cavium/liquidio/lio_main.c index 715ddfa..1126422 100644 --- a/drivers/net/ethernet/cavium/liquidio/lio_main.c +++ b/drivers/net/ethernet/cavium/liquidio/lio_main.c @@ -2354,7 +2354,7 @@ static void liquidio_set_mcast_list(struct net_device *netdev) struct octnic_ctrl_pkt nctrl; struct netdev_hw_addr *ha; u64 *mc; - int ret, i; + int ret; int mc_count = min(netdev_mc_count(netdev), MAX_OCTEON_MULTICAST_ADDR); memset(&nctrl, 0, sizeof(struct octnic_ctrl_pkt)); @@ -2370,7 +2370,6 @@ static void liquidio_set_mcast_list(struct net_device *netdev) nctrl.cb_fn = liquidio_link_ctrl_cmd_completion; /* copy all the addresses into the udd */ - i = 0; mc = &nctrl.udd[0]; netdev_for_each_mc_addr(ha, netdev) { *mc = 0;
[patch 3/3 -mainline] liquidio: off by one in liquidio_set_mcast_list()
The nctrl.udd[] array has 32 elements of size u64. Imagine that netdev_mc_count() returns more than 32. That means "mc_count" is 32. On the last iteration through the loop we have mc == &nctrl.udd[32] so we're writing one element beyond the end of the array. Fixes: f21fb3ed364b ('Add support of Cavium Liquidio ethernet adapters') Signed-off-by: Dan Carpenter diff --git a/drivers/net/ethernet/cavium/liquidio/lio_main.c b/drivers/net/ethernet/cavium/liquidio/lio_main.c index 1126422..41ee8bd 100644 --- a/drivers/net/ethernet/cavium/liquidio/lio_main.c +++ b/drivers/net/ethernet/cavium/liquidio/lio_main.c @@ -2376,7 +2376,7 @@ static void liquidio_set_mcast_list(struct net_device *netdev) memcpy(((u8 *)mc) + 2, ha->addr, ETH_ALEN); /* no need to swap bytes */ - if (++mc > &nctrl.udd[mc_count]) + if (++mc >= &nctrl.udd[mc_count]) break; }
[patch] qlcnic: use the correct ring in qlcnic_83xx_process_rcv_ring_diag()
There is a static checker warning here "warn: mask and shift to zero" and the code sets "ring" to zero every time. From looking at how QLCNIC_FETCH_RING_ID() is used in qlcnic_83xx_process_rcv_ring() the qlcnic_83xx_hndl() should be removed. Fixes: 4be41e92f7c6 ('qlcnic: 83xx data path routines') Signed-off-by: Dan Carpenter --- Not tested. diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c index 7bd6f25..607bb7d 100644 --- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c +++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c @@ -2220,7 +2220,7 @@ void qlcnic_83xx_process_rcv_ring_diag(struct qlcnic_host_sds_ring *sds_ring) if (!opcode) return; - ring = QLCNIC_FETCH_RING_ID(qlcnic_83xx_hndl(sts_data[0])); + ring = QLCNIC_FETCH_RING_ID(sts_data[0]); qlcnic_83xx_process_rcv_diag(adapter, ring, sts_data); desc = &sds_ring->desc_head[consumer]; desc->status_desc_data[0] = cpu_to_le64(STATUS_OWNER_PHANTOM);
[patch] be2net: signedness bug in be_msix_enable()
"num_vec" needs to be signed for the error handling to work. Fixes: e261768e9e39 ('be2net: support asymmetric rx/tx queue counts') Signed-off-by: Dan Carpenter diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c index 1873c74..1f16e73 100644 --- a/drivers/net/ethernet/emulex/benet/be_main.c +++ b/drivers/net/ethernet/emulex/benet/be_main.c @@ -3251,8 +3251,9 @@ static void be_msix_disable(struct be_adapter *adapter) static int be_msix_enable(struct be_adapter *adapter) { - unsigned int i, num_vec, max_roce_eqs; + unsigned int i, max_roce_eqs; struct device *dev = &adapter->pdev->dev; + int num_vec; /* If RoCE is supported, program the max number of vectors that * could be used for NIC and RoCE, else, just program the number
[patch] qlcnic: fix a loop exit condition better
In the original code, if we succeeded on the last iteration through the loop then we still returned failure. Fixes: 389e4e04ad2d ('qlcnic: fix a timeout loop') Signed-off-by: Dan Carpenter diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_vnic.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_vnic.c index b1a452f..3490675 100644 --- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_vnic.c +++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_vnic.c @@ -252,7 +252,7 @@ int qlcnic_83xx_check_vnic_state(struct qlcnic_adapter *adapter) state = QLCRDX(ahw, QLC_83XX_VNIC_STATE); } - if (!idc->vnic_wait_limit) { + if (state != QLCNIC_DEV_NPAR_OPER) { dev_err(&adapter->pdev->dev, "vNIC mode not operational, state check timed out.\n"); return -EIO; -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] rsi: Delete unnecessary variable initialisations in rsi_send_mgmt_pkt()
These patches are labour intensive to review because you can't just do it in the email client. Also you were not able to review it properly yourself and introduced a bug. I am often remove initializers but it's normally because I am changing something else which makes it worthwhile. This patch is the correct thing but it's not "worthwhile". It is not a good use of my time. Please stop sending cleanup patches, Markus. Just send fixes. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] rsi: Delete unnecessary variable initialisations in rsi_send_mgmt_pkt()
Btw, GCC misses a lot of uninitialized variable bugs. I have a Smatch check which sometimes catches the bugs that GCC misses but you should not rely on the tools here. These patches need to be reviewed manually. And the "goto err" before the initialization makes everything more complicated (that's actually what caused the bug in this patch, in fact). regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/5] xen-netback: Fine-tuning for three function implementations
The original code is fine. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] rsi: Delete unnecessary variable initialisations in rsi_send_mgmt_pkt()
On Mon, Jan 04, 2016 at 11:44:15AM +0100, SF Markus Elfring wrote: > > Please stop sending cleanup patches, Markus. Just send fixes. > > How often will source code clean-up fix something? > > > May I resend a consistent patch series for the source file > "drivers/net/wireless/rsi/rsi_91x_pkt.c" in the near future? If you were sending checkpatch.pl fixes that would be easier to deal with but you are sending hundreds of "controversial" cleanups. They are controversial in the sense that they don't fix anything against official kernel style and they go against the author's original intention. I tend to agree that useless initializers are bad and disable GCCs uninitialized variable warnings but just because I agree with you doesn't make it official kernel style. It's slightly rude to go against the author's intention. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] rsi: Delete unnecessary variable initialisations in rsi_send_mgmt_pkt()
On Mon, Jan 04, 2016 at 02:17:40PM +0100, Bjørn Mork wrote: > Dan Carpenter writes: > > > Please stop sending cleanup patches, Markus. Just send fixes. > > Thanks for your continued but unwarranted belief in AI. > I always tell people that I am very mechanical and you can rely on me to send predictable responses... > Do you mind if I remind you of https://lkml.org/lkml/2014/11/3/162 ? > I am sure there are lots and lots of other examples. There is no reason > to believe this will ever stop. He just goes into Eliza mode. Yup. I feel some sense of responsibility for any patches where kernel-janitors is on the CC but I'm having a hard time dealing with all of Markus's patches. Normally you just respond to the first patch and people change the later patches but, as you put it, Markus just goes into ELIZA mode. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch v2] qlcnic: fix a timeout loop
No. This patch is a suspend resume thing and your bug is something else. Honestly, this patch is a static checker fix and I doubt it has much real worl impact at all. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch -next] mlxsw: core: remove an unnecessary condition
We checked "err" on the lines before so we know it's zero here. These cause a static checker warning because checking known things can indicate a bug. Maybe there is a missing assignment or we are checking the wrong variable. Signed-off-by: Dan Carpenter diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_hwmon.c b/drivers/net/ethernet/mellanox/mlxsw/core_hwmon.c index 5b9364f..1ac8bf1 100644 --- a/drivers/net/ethernet/mellanox/mlxsw/core_hwmon.c +++ b/drivers/net/ethernet/mellanox/mlxsw/core_hwmon.c @@ -130,7 +130,7 @@ static ssize_t mlxsw_hwmon_temp_rst_store(struct device *dev, dev_err(mlxsw_hwmon->bus_info->dev, "Failed to reset temp sensor history\n"); return err; } - return err ? err : len; + return len; } static ssize_t mlxsw_hwmon_fan_rpm_show(struct device *dev, -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch -next] fsl/fman: fix the pause_time test
pause_time is unsigned so it can't be less than zero. The bug means that we allow invalid pause-times. Fixes: 57ba4c9b56d8 ('fsl/fman: Add FMan MAC support') Signed-off-by: Dan Carpenter diff --git a/drivers/net/ethernet/freescale/fman/fman_dtsec.c b/drivers/net/ethernet/freescale/fman/fman_dtsec.c index d78e2ba..587f9b4 100644 --- a/drivers/net/ethernet/freescale/fman/fman_dtsec.c +++ b/drivers/net/ethernet/freescale/fman/fman_dtsec.c @@ -934,7 +934,7 @@ int dtsec_set_tx_pause_frames(struct fman_mac *dtsec, /* FM_BAD_TX_TS_IN_B_2_B_ERRATA_DTSEC_A003 Errata workaround */ if (dtsec->fm_rev_info.major == 2) - if (pause_time < 0 && pause_time <= 320) { + if (pause_time <= 320) { pr_warn("pause-time: %d illegal.Should be > 320\n", pause_time); return -EINVAL; -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch -next] fsl/fman: double free on probe failure
"priv" is allocated with devm_kzalloc() so freeing it here with kfree() will lead to a double free. Fixes: 3933961682a3 ('fsl/fman: Add FMan MAC driver') Signed-off-by: Dan Carpenter diff --git a/drivers/net/ethernet/freescale/fman/mac.c b/drivers/net/ethernet/freescale/fman/mac.c index 743a393..e33d9d2 100644 --- a/drivers/net/ethernet/freescale/fman/mac.c +++ b/drivers/net/ethernet/freescale/fman/mac.c @@ -961,7 +961,6 @@ _return_of_node_put: of_node_put(dev_node); _return_dev_set_drvdata: kfree(priv->fixed_link); - kfree(priv); dev_set_drvdata(dev, NULL); _return: return err; -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch -next] fsl/fman: use the ALIGN() macro
The original code works fine but the issue is that static checkers complain about this: ~(u16)(OFFSET_UNITS - 1) In that expression the cast to u16 is a no-op because we cast the value 15 to a u16 but then type promotion rules automatically cast it back to an int and then we do the bitwise negate. It's cleaner to use the kernel's ALIGN() macro instead. Signed-off-by: Dan Carpenter --- I think this patch is correct, but I haven't tested it, so please review carefully. diff --git a/drivers/net/ethernet/freescale/fman/fman_sp.c b/drivers/net/ethernet/freescale/fman/fman_sp.c index f9e7aa3..b527da1 100644 --- a/drivers/net/ethernet/freescale/fman/fman_sp.c +++ b/drivers/net/ethernet/freescale/fman/fman_sp.c @@ -92,11 +92,8 @@ int fman_sp_build_buffer_struct(struct fman_sp_int_context_data_copy * u32 tmp; /* Align start of internal context data to 16 byte */ - int_context_data_copy->ext_buf_offset = (u16) - ((buffer_prefix_content->priv_data_size & (OFFSET_UNITS - 1)) ? - ((buffer_prefix_content->priv_data_size + OFFSET_UNITS) & - ~(u16)(OFFSET_UNITS - 1)) : - buffer_prefix_content->priv_data_size); + int_context_data_copy->ext_buf_offset = + (u16)ALIGN(buffer_prefix_content->priv_data_size, 16); /* Translate margin and int_context params to FM parameters */ /* Initialize with illegal value. Later we'll set legal values. */ -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch -next] fsl/fman: use the ALIGN() macro
On Wed, Jan 06, 2016 at 03:27:38PM -0500, David Miller wrote: > From: Dan Carpenter > Date: Wed, 6 Jan 2016 13:03:08 +0300 > > > diff --git a/drivers/net/ethernet/freescale/fman/fman_sp.c > > b/drivers/net/ethernet/freescale/fman/fman_sp.c > > index f9e7aa3..b527da1 100644 > > --- a/drivers/net/ethernet/freescale/fman/fman_sp.c > > +++ b/drivers/net/ethernet/freescale/fman/fman_sp.c > > @@ -92,11 +92,8 @@ int fman_sp_build_buffer_struct(struct > > fman_sp_int_context_data_copy * > > u32 tmp; > > > > /* Align start of internal context data to 16 byte */ > > - int_context_data_copy->ext_buf_offset = (u16) > > - ((buffer_prefix_content->priv_data_size & (OFFSET_UNITS - 1)) ? > > - ((buffer_prefix_content->priv_data_size + OFFSET_UNITS) & > > - ~(u16)(OFFSET_UNITS - 1)) : > > - buffer_prefix_content->priv_data_size); > > + int_context_data_copy->ext_buf_offset = > > + (u16)ALIGN(buffer_prefix_content->priv_data_size, 16); > > Why are you using '16' instead of OFFSET_UNITS? I made a brain fart. :/ I'll resend. Thanks. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] irda: precedence bug in irlmp_seq_hb_idx()
This is decrementing the pointer, instead of the value stored in the pointer. KASan detects it as an out of bounds reference. Reported-by: "Berry Cheng 程君(成淼)" Signed-off-by: Dan Carpenter --- This bug predates the start of git. You would think it would have been reported earlier since it looks like a serious bug. I cannot test this so please review carefully. diff --git a/net/irda/irlmp.c b/net/irda/irlmp.c index a26c401..4396459 100644 --- a/net/irda/irlmp.c +++ b/net/irda/irlmp.c @@ -1839,7 +1839,7 @@ static void *irlmp_seq_hb_idx(struct irlmp_iter_state *iter, loff_t *off) for (element = hashbin_get_first(iter->hashbin); element != NULL; element = hashbin_get_next(iter->hashbin)) { - if (!off || *off-- == 0) { + if (!off || (*off)-- == 0) { /* NB: hashbin left locked */ return element; } -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] NFC: delete null dereference
The next goto after that is messed up as well: 1056 dev = nfc_get_device(idx); 1057 if (!dev) 1058 return -ENODEV; 1059 1060 device_lock(&dev->dev); 1061 1062 local = nfc_llcp_find_local(dev); 1063 if (!local) { 1064 nfc_put_device(dev); It should not call nfc_put_device() because that happens after goto exit. 1065 rc = -ENODEV; 1066 goto exit; 1067 } regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch -next] qlogic/qed: remove bogus NULL check
We check if "p_hwfn" is NULL and then dereference it in the error handling code. I read the code and it isn't NULL so let's remove the check. Signed-off-by: Dan Carpenter diff --git a/drivers/net/ethernet/qlogic/qed/qed_int.c b/drivers/net/ethernet/qlogic/qed/qed_int.c index 2e399b6..de50e84 100644 --- a/drivers/net/ethernet/qlogic/qed/qed_int.c +++ b/drivers/net/ethernet/qlogic/qed/qed_int.c @@ -251,11 +251,6 @@ void qed_int_sp_dpc(unsigned long hwfn_cookie) int arr_size; u16 rc = 0; - if (!p_hwfn) { - DP_ERR(p_hwfn->cdev, "DPC called - no hwfn!\n"); - return; - } - if (!p_hwfn->p_sp_sb) { DP_ERR(p_hwfn->cdev, "DPC called - no p_sp_sb\n"); return; -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch -next] qlogic: qed: fix a test for MODE_MF_SI
MODE_MF_SI is 9. We should be testing bit 9 instead of AND 0x9. Fixes: fe56b9e6a8d9 ('qed: Add module with basic common support') Signed-off-by: Dan Carpenter diff --git a/drivers/net/ethernet/qlogic/qed/qed_dev.c b/drivers/net/ethernet/qlogic/qed/qed_dev.c index b9b7b7e..774b223 100644 --- a/drivers/net/ethernet/qlogic/qed/qed_dev.c +++ b/drivers/net/ethernet/qlogic/qed/qed_dev.c @@ -562,7 +562,7 @@ static int qed_hw_init_pf(struct qed_hwfn *p_hwfn, } /* Enable classification by MAC if needed */ - if (hw_mode & MODE_MF_SI) { + if (hw_mode & (1 << MODE_MF_SI)) { DP_VERBOSE(p_hwfn, NETIF_MSG_HW, "Configuring TAGMAC_CLS_TYPE\n"); STORE_RT_REG(p_hwfn, -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch -next] qlogic: qed: fix error codes in qed_resc_alloc()
We accidentally return success instead of -ENOMEM here. Fixes: fe56b9e6a8d9 ('qed: Add module with basic common support') Signed-off-by: Dan Carpenter diff --git a/drivers/net/ethernet/qlogic/qed/qed_dev.c b/drivers/net/ethernet/qlogic/qed/qed_dev.c index b9b7b7e..562e160 100644 --- a/drivers/net/ethernet/qlogic/qed/qed_dev.c +++ b/drivers/net/ethernet/qlogic/qed/qed_dev.c @@ -223,6 +223,7 @@ int qed_resc_alloc(struct qed_dev *cdev) if (!p_hwfn->p_tx_cids) { DP_NOTICE(p_hwfn, "Failed to allocate memory for Tx Cids\n"); + rc = -ENOMEM; goto alloc_err; } @@ -230,6 +231,7 @@ int qed_resc_alloc(struct qed_dev *cdev) if (!p_hwfn->p_rx_cids) { DP_NOTICE(p_hwfn, "Failed to allocate memory for Rx Cids\n"); + rc = -ENOMEM; goto alloc_err; } } @@ -281,14 +283,17 @@ int qed_resc_alloc(struct qed_dev *cdev) /* EQ */ p_eq = qed_eq_alloc(p_hwfn, 256); - - if (!p_eq) + if (!p_eq) { + rc = -ENOMEM; goto alloc_err; + } p_hwfn->p_eq = p_eq; p_consq = qed_consq_alloc(p_hwfn); - if (!p_consq) + if (!p_consq) { + rc = -ENOMEM; goto alloc_err; + } p_hwfn->p_consq = p_consq; /* DMA info initialization */ @@ -303,6 +308,7 @@ int qed_resc_alloc(struct qed_dev *cdev) cdev->reset_stats = kzalloc(sizeof(*cdev->reset_stats), GFP_KERNEL); if (!cdev->reset_stats) { DP_NOTICE(cdev, "Failed to allocate reset statistics\n"); + rc = -ENOMEM; goto alloc_err; } -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] netlink: fix a limit in NETLINK_LIST_MEMBERSHIPS
This condition doesn't work when len is smaller than expected and not a multiple of 4. In that situation "len - pos" is negative and type promoted to a high unsigned value and we do not break out of the loop. It causes the program calling it to crash. Fixes: b42be38b2778 ('netlink: add API to retrieve all group memberships') Signed-off-by: Dan Carpenter diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c index 59651af..76a8466 100644 --- a/net/netlink/af_netlink.c +++ b/net/netlink/af_netlink.c @@ -2373,7 +2373,7 @@ static int netlink_getsockopt(struct socket *sock, int level, int optname, err = 0; netlink_lock_table(); for (pos = 0; pos * 8 < nlk->ngroups; pos += sizeof(u32)) { - if (len - pos < sizeof(u32)) + if (len < pos + sizeof(u32)) break; idx = pos / sizeof(unsigned long); -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] netlink: fix a limit in NETLINK_LIST_MEMBERSHIPS
Oh. Crap... My mistake. Sorry for the noise. The original code is fine. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] net/hsr: fix a warning message
WARN_ON_ONCE() takes a condition, it doesn't take an error message. I have converted this to WARN() instead. Signed-off-by: Dan Carpenter diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c index 35a9788..c7d1adc 100644 --- a/net/hsr/hsr_device.c +++ b/net/hsr/hsr_device.c @@ -312,7 +312,7 @@ static void send_hsr_supervision_frame(struct hsr_port *master, u8 type) return; out: - WARN_ON_ONCE("HSR: Could not send supervision frame\n"); + WARN_ONCE(1, "HSR: Could not send supervision frame\n"); kfree_skb(skb); } -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] staging: rtl8712: Remove casts between void * and type *
On Tue, Nov 24, 2015 at 10:19:39AM -0200, Mauro Dreissig wrote: > Cleaning rtl871x_ioctl_rtl.c. > It's better if you think about the header and the body as two different things. Just repeat the title but with more information. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 1/2 -mainline] cxgb4: missing curly braces in t4_setup_debugfs()
There were missing curly braces so it means we call add_debugfs_mem() unintentionally. Fixes: 3ccc6cf74d8c ('cxgb4: Adds support for T6 adapter') Signed-off-by: Dan Carpenter diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c index b657734..9e0b670 100644 --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c @@ -2665,10 +2665,11 @@ int t4_setup_debugfs(struct adapter *adap) EXT_MEM1_SIZE_G(size)); } } else { - if (i & EXT_MEM_ENABLE_F) + if (i & EXT_MEM_ENABLE_F) { size = t4_read_reg(adap, MA_EXT_MEMORY_BAR_A); add_debugfs_mem(adap, "mc", MEM_MC, EXT_MEM_SIZE_G(size)); + } } de = debugfs_create_file_size("flash", S_IRUSR, adap->debugfs_root, adap, -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 2/2] cxgb4: cleanup some indenting
Add or remove some tabs so that statements line up correctly. Signed-off-by: Dan Carpenter diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c index 9e0b670..b83ca7f 100644 --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c @@ -346,11 +346,11 @@ static int cim_qcfg_show(struct seq_file *seq, void *v) if (is_t4(adap->params.chip)) { i = t4_cim_read(adap, UP_OBQ_0_REALADDR_A, ARRAY_SIZE(obq_wr_t4), obq_wr_t4); - wr = obq_wr_t4; + wr = obq_wr_t4; } else { i = t4_cim_read(adap, UP_OBQ_0_SHADOW_REALADDR_A, ARRAY_SIZE(obq_wr_t5), obq_wr_t5); - wr = obq_wr_t5; + wr = obq_wr_t5; } } if (i) @@ -2095,7 +2095,7 @@ do { \ #undef T #undef S #undef S3 -return 0; + return 0; } static int sge_queue_entries(const struct adapter *adap) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] hamradio/kiss: missing error code in mkiss_open()
If register_netdev() fails we return success but we should return an error code instead. Reported-by: RUC_Soft_Sec Signed-off-by: Dan Carpenter diff --git a/drivers/net/hamradio/mkiss.c b/drivers/net/hamradio/mkiss.c index 2ffbf13..dcb6bb7 100644 --- a/drivers/net/hamradio/mkiss.c +++ b/drivers/net/hamradio/mkiss.c @@ -732,7 +732,8 @@ static int mkiss_open(struct tty_struct *tty) goto out_free_netdev; } - if (register_netdev(dev)) + err = register_netdev(dev); + if (err) goto out_free_buffers; /* after register_netdev() - because else printk smashes the kernel */ -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] cosa: missing error code on failure in probe()
If register_hdlc_device() fails, the current code returns 0 but we should return an error code instead. Signed-off-by: Dan Carpenter diff --git a/drivers/net/wan/cosa.c b/drivers/net/wan/cosa.c index 7193b73..848ea6a 100644 --- a/drivers/net/wan/cosa.c +++ b/drivers/net/wan/cosa.c @@ -589,7 +589,8 @@ static int cosa_probe(int base, int irq, int dma) chan->netdev->base_addr = chan->cosa->datareg; chan->netdev->irq = chan->cosa->irq; chan->netdev->dma = chan->cosa->dma; - if (register_hdlc_device(chan->netdev)) { + err = register_hdlc_device(chan->netdev); + if (err) { netdev_warn(chan->netdev, "register_hdlc_device() failed\n"); free_netdev(chan->netdev); -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch -next] bpf: off by one in check_map_func_compatibility()
The loop iterates one space too far, so we might read beyond the end of the func_limit[] array. Fixes: 35578d798400 ('bpf: Implement function bpf_perf_event_read() that get the selected hardware PMU conuter') Signed-off-by: Dan Carpenter diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 48e1c71..ed12e38 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -853,7 +853,7 @@ static int check_map_func_compatibility(struct bpf_map *map, int func_id) if (!map) return 0; - for (i = 0; i <= ARRAY_SIZE(func_limit); i++) { + for (i = 0; i < ARRAY_SIZE(func_limit); i++) { bool_map = (map->map_type == func_limit[i].map_type); bool_func = (func_id == func_limit[i].func_id); /* only when map & func pair match it can continue. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] net: ethernet: micrel: fix an error code
The dma_mapping_error() function returns true or false. We should return -ENOMEM if it there is a dma mapping error. Signed-off-by: Dan Carpenter diff --git a/drivers/net/ethernet/micrel/ks8842.c b/drivers/net/ethernet/micrel/ks8842.c index f78909a..09d2e16 100644 --- a/drivers/net/ethernet/micrel/ks8842.c +++ b/drivers/net/ethernet/micrel/ks8842.c @@ -952,9 +952,8 @@ static int ks8842_alloc_dma_bufs(struct net_device *netdev) sg_dma_address(&tx_ctl->sg) = dma_map_single(adapter->dev, tx_ctl->buf, DMA_BUFFER_SIZE, DMA_TO_DEVICE); - err = dma_mapping_error(adapter->dev, - sg_dma_address(&tx_ctl->sg)); - if (err) { + if (dma_mapping_error(adapter->dev, sg_dma_address(&tx_ctl->sg))) { + err = -ENOMEM; sg_dma_address(&tx_ctl->sg) = 0; goto err; } -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] cxgb4: memory corruption in debugfs
You can't use kstrtoul() with an int or it causes memory corruption. Also j should be unsigned or we have underflow bugs. I considered changing "j" to unsigned long but everything fits in a u32. Fixes: 8e3d04fd7d70 ('cxgb4: Add MPS tracing support') Signed-off-by: Dan Carpenter diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c index 1732e29..0a87a32 100644 --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c @@ -1289,13 +1289,14 @@ static unsigned int xdigit2int(unsigned char c) static ssize_t mps_trc_write(struct file *file, const char __user *buf, size_t count, loff_t *pos) { - int i, j, enable, ret; + int i, enable, ret; u32 *data, *mask; struct trace_params tp; const struct inode *ino; unsigned int trcidx; char *s, *p, *word, *end; struct adapter *adap; + u32 j; ino = file_inode(file); trcidx = (uintptr_t)ino->i_private & 3; @@ -1340,7 +1341,7 @@ static ssize_t mps_trc_write(struct file *file, const char __user *buf, if (!strncmp(word, "qid=", 4)) { end = (char *)word + 4; - ret = kstrtoul(end, 10, (unsigned long *)&j); + ret = kstrtouint(end, 10, &j); if (ret) goto out; if (!adap->trace_rss) { @@ -1369,7 +1370,7 @@ static ssize_t mps_trc_write(struct file *file, const char __user *buf, } if (!strncmp(word, "snaplen=", 8)) { end = (char *)word + 8; - ret = kstrtoul(end, 10, (unsigned long *)&j); + ret = kstrtouint(end, 10, &j); if (ret || j > 9600) { inval: count = -EINVAL; goto out; @@ -1379,7 +1380,7 @@ inval:count = -EINVAL; } if (!strncmp(word, "minlen=", 7)) { end = (char *)word + 7; - ret = kstrtoul(end, 10, (unsigned long *)&j); + ret = kstrtouint(end, 10, &j); if (ret || j > TFMINPKTSIZE_M) goto inval; tp.min_len = j; @@ -1453,7 +1454,7 @@ inval:count = -EINVAL; } if (*word == '@') { end = (char *)word + 1; - ret = kstrtoul(end, 10, (unsigned long *)&j); + ret = kstrtouint(end, 10, &j); if (*end && *end != '\n') goto inval; if (j & 7) /* doesn't start at multiple of 8 */ -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] cxgb4: memory corruption in debugfs
On Tue, Aug 18, 2015 at 07:28:53PM +0900, Tetsuo Handa wrote: > Dan Carpenter wrote: > > You can't use kstrtoul() with an int or it causes memory corruption. > > Also j should be unsigned or we have underflow bugs. > > > > I considered changing "j" to unsigned long but everything fits in a u32. > > Excuse me, but kstrtouint()'s last argument is not "u32 *" but "unsigned int > *". > Aren't there architectures where sizeof(unsigned int) > sizeof(u32) ? No. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch - Nicholas's tree] netconsole: Missing unlock on error path
We added new locking to this function but we missed one error path which needs an unlock. Fixes: cdacad4993f4 ('netconsole: use per-attribute show and store methods') Signed-off-by: Dan Carpenter --- This is going through Nicholas Bellinger's tree not net-next. diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c index 8783169..06ee639 100644 --- a/drivers/net/netconsole.c +++ b/drivers/net/netconsole.c @@ -535,7 +535,7 @@ static ssize_t remote_ip_store(struct config_item *item, const char *buf, if (in6_pton(buf, count, nt->np.remote_ip.in6.s6_addr, -1, &end) > 0) { if (*end && *end != '\n') { pr_err("invalid IPv6 address at: <%c>\n", *end); - return -EINVAL; + goto out_unlock; } nt->np.ipv6 = true; } else -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V3 3/7] Drivers: hv: vmbus: add APIs to send/recv hvsock packet and get the r/w-ability
On Wed, Jul 22, 2015 at 10:09:10AM +, Dexuan Cui wrote: > > I'd suggest you do something like > > > > if (ret == -EAGIAIN) > > return 0; > > else if (ret) > > return ret; > > > > to make it future-proof (e.g. when a new error is returned by > > hv_ringbuffer_peek). And a comment would also be useful as it is unclear > > why we silence errors here. > Hi Vitaly, > Thanks! > I think I made a mistake here: > the "if (ret != 0) return 0;" should be changed > to "if (ret != 0) return ret;" The double negative really doesn't not make the code more complicated. I like using a quadruple negative instead. if (ret != 0 != 0) return ret; regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V3 3/7] Drivers: hv: vmbus: add APIs to send/recv hvsock packet and get the r/w-ability
On Thu, Jul 23, 2015 at 03:05:16AM +, Dexuan Cui wrote: > The kind of usage is not rare in the kernel code: Yeah. But it's used 5% of the time. If it's under 15% then there is a risk that we'll write a checkpatch rule to enforce the standard way... There are some places where != 0 is idiomatic, like when you are talking about the number zero. strcmp() and friends should always be != 0 or == 0. In this specific case, writing it as "if (ret != 0)" caused the bug. If we had written it as "if (ret) return ret;" then there are no zeroes so wouldn't have been any temptation to return the zero instead of the ret. > Hi Dan, I read this as a humor. :-) :) regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V3 3/7] Drivers: hv: vmbus: add APIs to send/recv hvsock packet and get the r/w-ability
On Thu, Jul 23, 2015 at 01:10:57PM +0300, Dan Carpenter wrote: > In this specific case, writing it as "if (ret != 0)" caused the bug. If > we had written it as "if (ret) return ret;" then there are no zeroes so > wouldn't have been any temptation to return the zero instead of the ret. I did a search to see if returning the zero instead of the ret was a common mistake and it seems like it might be. I did: grep 'if (ret != 0)' drivers/ -r -A1 -n | grep "return 0;" | perl -ne 's/.c-(\d+)-/.c:$1/; print' drivers/gpu/drm/nouveau/nouveau_display.c:111 return 0; drivers/regulator/wm8400-regulator.c:47 return 0; drivers/platform/x86/dell-laptop.c:859 return 0; drivers/media/dvb-frontends/dibx000_common.c:213 return 0; drivers/media/dvb-frontends/dibx000_common.c:217 return 0; drivers/media/dvb-frontends/dibx000_common.c:235 return 0; drivers/media/dvb-frontends/dibx000_common.c:239 return 0; drivers/hv/channel.c:898return 0; drivers/hv/channel.c:944return 0; A bunch of those look suspicious but I don't know the subsystems well enough to be sure. Can you check the last two? regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V3 3/7] Drivers: hv: vmbus: add APIs to send/recv hvsock packet and get the r/w-ability
On Fri, Jul 24, 2015 at 11:57:01AM +0530, Sudip Mukherjee wrote: > This is also ok, the function is supposed to return ret or-ed with the > relevant flags based on the scan position. It is considered error if 0 > is returned (without any flag). Yeah. You're right. I looked through my list again this morning and they all seem fine... regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch -next] lwtunnel: use kfree_skb() instead of vanilla kfree()
kfree_skb() is correct here. Fixes: ffce41962ef6 ('lwtunnel: support dst output redirect function') Signed-off-by: Dan Carpenter diff --git a/net/core/lwtunnel.c b/net/core/lwtunnel.c index bb58826..5f7fae7 100644 --- a/net/core/lwtunnel.c +++ b/net/core/lwtunnel.c @@ -205,7 +205,7 @@ int __lwtunnel_output(struct sock *sk, struct sk_buff *skb, return ret; drop: - kfree(skb); + kfree_skb(skb); return ret; } -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] packet: missing dev_put() in packet_do_bind()
From: Lars Westerhoff When binding a PF_PACKET socket, the use count of the bound interface is always increased with dev_hold in dev_get_by_{index,name}. However, when rebound with the same protocol and device as in the previous bind the use count of the interface was not decreased. Ultimately, this caused the deletion of the interface to fail with the following message: unregister_netdevice: waiting for dummy0 to become free. Usage count = 1 This patch moves the dev_put out of the conditional part that was only executed when either the protocol or device changed on a bind. Fixes: 902fefb82ef7 ('packet: improve socket create/bind latency in some cases') Signed-off-by: Lars Westerhoff Signed-off-by: Dan Carpenter Reviewed-by: Daniel Borkmann diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index c9e8741..c7c42eb 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -2784,7 +2784,7 @@ static int packet_release(struct socket *sock) static int packet_do_bind(struct sock *sk, struct net_device *dev, __be16 proto) { struct packet_sock *po = pkt_sk(sk); - const struct net_device *dev_curr; + struct net_device *dev_curr; __be16 proto_curr; bool need_rehook; @@ -2808,15 +2808,13 @@ static int packet_do_bind(struct sock *sk, struct net_device *dev, __be16 proto) po->num = proto; po->prot_hook.type = proto; - - if (po->prot_hook.dev) - dev_put(po->prot_hook.dev); - po->prot_hook.dev = dev; po->ifindex = dev ? dev->ifindex : 0; packet_cached_dev_assign(po, dev); } + if (dev_curr) + dev_put(dev_curr); if (proto == 0 || !need_rehook) goto out_unlock; -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch -master] netfilter: xt_CT: checking for IS_ERR() instead of NULL
We recently changed this from nf_conntrack_alloc() to nf_ct_tmpl_alloc() so the error handling needs to changed to check for NULL instead of IS_ERR(). Fixes: 0838aa7fcfcd ('netfilter: fix netns dependencies with conntrack templates') Signed-off-by: Dan Carpenter diff --git a/net/netfilter/xt_CT.c b/net/netfilter/xt_CT.c index c663003..43ddeee 100644 --- a/net/netfilter/xt_CT.c +++ b/net/netfilter/xt_CT.c @@ -202,9 +202,10 @@ static int xt_ct_tg_check(const struct xt_tgchk_param *par, goto err1; ct = nf_ct_tmpl_alloc(par->net, info->zone, GFP_KERNEL); - ret = PTR_ERR(ct); - if (IS_ERR(ct)) + if (!ct) { + ret = -ENOMEM; goto err2; + } ret = 0; if ((info->ct_events || info->exp_events) && -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch -master] netfilter: xt_CT: checking for IS_ERR() instead of NULL
On Thu, Jul 30, 2015 at 01:57:43PM +0200, Pablo Neira Ayuso wrote: > I have also appended this chunk, since synproxy is also affected: Thanks. I have rechecked my scripts and I *should* have caught that. I'm not sure what went wrong... regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] rds: fix an integer overflow test in rds_info_getsockopt()
"len" is a signed integer. We check that len is not negative, so it goes from zero to INT_MAX. PAGE_SIZE is unsigned long so the comparison is type promoted to unsigned long. ULONG_MAX - 4095 is a higher than INT_MAX so the condition can never be true. I don't know if this is harmful but it seems safe to limit "len" to INT_MAX - 4095. Fixes: a8c879a7ee98 ('RDS: Info and stats') Signed-off-by: Dan Carpenter diff --git a/net/rds/info.c b/net/rds/info.c index 9a6b4f6..140a44a 100644 --- a/net/rds/info.c +++ b/net/rds/info.c @@ -176,7 +176,7 @@ int rds_info_getsockopt(struct socket *sock, int optname, char __user *optval, /* check for all kinds of wrapping and the like */ start = (unsigned long)optval; - if (len < 0 || len + PAGE_SIZE - 1 < len || start + len < start) { + if (len < 0 || len > INT_MAX - PAGE_SIZE + 1 || start + len < start) { ret = -EINVAL; goto out; } -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
re: af_mpls: fix undefined reference to ip6_route_output
Hello Roopa, I have a concern about patch bf21563acc1d: "af_mpls: fix undefined reference to ip6_route_output" from Jul 30, 2015. net/mpls/af_mpls.c 450 451 dev = find_outdev(net, cfg); 452 if (IS_ERR(dev)) { find_outdev() used to return NULL pointers but now it only returns NULL if cfg->rc_via_table == NEIGH_LINK_TABLE or dev_get_by_index() fails. I think it could lead to a NULL dereference and we don't check for that here. Returning a mix of error pointers and NULL is bad style, it needs a big comment at the top of the function if it's deliberate. 453 err = PTR_ERR(dev); 454 dev = NULL; 455 goto errout; 456 } regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] mpls: small cleanup in inet/inet6_fib_lookup_dev()
We recently changed this code from returning NULL to returning ERR_PTR. There are some left over NULL assignments which we can remove. We can preserve the error code from ip_route_output() instead of always returning -ENODEV. Also these functions use a mix of gotos and direct returns. There is no cleanup necessary so I changed the gotos to direct returns. Signed-off-by: Dan Carpenter diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c index 88cfaa2..d933059 100644 --- a/net/mpls/af_mpls.c +++ b/net/mpls/af_mpls.c @@ -337,14 +337,14 @@ static unsigned find_free_label(struct net *net) #if IS_ENABLED(CONFIG_INET) static struct net_device *inet_fib_lookup_dev(struct net *net, void *addr) { - struct net_device *dev = NULL; + struct net_device *dev; struct rtable *rt; struct in_addr daddr; memcpy(&daddr, addr, sizeof(struct in_addr)); rt = ip_route_output(net, daddr.s_addr, 0, 0, 0); if (IS_ERR(rt)) - goto errout; + return ERR_CAST(rt); dev = rt->dst.dev; dev_hold(dev); @@ -352,8 +352,6 @@ static struct net_device *inet_fib_lookup_dev(struct net *net, void *addr) ip_rt_put(rt); return dev; -errout: - return ERR_PTR(-ENODEV); } #else static struct net_device *inet_fib_lookup_dev(struct net *net, void *addr) @@ -365,7 +363,7 @@ static struct net_device *inet_fib_lookup_dev(struct net *net, void *addr) #if IS_ENABLED(CONFIG_IPV6) static struct net_device *inet6_fib_lookup_dev(struct net *net, void *addr) { - struct net_device *dev = NULL; + struct net_device *dev; struct dst_entry *dst; struct flowi6 fl6; int err; @@ -377,16 +375,13 @@ static struct net_device *inet6_fib_lookup_dev(struct net *net, void *addr) memcpy(&fl6.daddr, addr, sizeof(struct in6_addr)); err = ipv6_stub->ipv6_dst_lookup(net, NULL, &dst, &fl6); if (err) - goto errout; + return ERR_PTR(err); dev = dst->dev; dev_hold(dev); dst_release(dst); return dev; - -errout: - return ERR_PTR(err); } #else static struct net_device *inet6_fib_lookup_dev(struct net *net, void *addr) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
re: fjes: update_zone_task
Hello Taku Izumi, The patch 785f28e061a8: "fjes: update_zone_task" from Aug 21, 2015, leads to the following static checker warning: drivers/net/fjes/fjes_hw.c:1016 fjes_hw_update_zone_task() warn: potential off by one 'info[]' limit 'hw->max_epid' drivers/net/fjes/fjes_hw.c 963 case 0: 964 965 for (epidx = 0; epidx < hw->max_epid; epidx++) { 966 if (epidx == hw->my_epid) { 967 hw->ep_shm_info[epidx].es_status = 968 info[epidx].es_status; 969 hw->ep_shm_info[epidx].zone = 970 info[epidx].zone; 971 continue; 972 } 973 974 pstatus = fjes_hw_get_partner_ep_status(hw, epidx); 975 switch (pstatus) { 976 case EP_PARTNER_UNSHARE: 977 default: 978 if ((info[epidx].zone != 979 FJES_ZONING_ZONE_TYPE_NONE) && 980 (info[epidx].es_status == 981 FJES_ZONING_STATUS_ENABLE) && 982 (info[epidx].zone == 983 info[hw->my_epid].zone)) 984 set_bit(epidx, &share_bit); 985 else 986 set_bit(epidx, &unshare_bit); 987 break; 988 989 case EP_PARTNER_COMPLETE: 990 case EP_PARTNER_WAITING: 991 if ((info[epidx].zone == 992 FJES_ZONING_ZONE_TYPE_NONE) || 993 (info[epidx].es_status != 994 FJES_ZONING_STATUS_ENABLE) || 995 (info[epidx].zone != 996 info[hw->my_epid].zone)) { 997 set_bit(epidx, 998 &adapter->unshare_watch_bitmask); 999 set_bit(epidx, 1000 &hw->hw_info.buffer_unshare_reserve_bit); 1001 } 1002 break; 1003 1004 case EP_PARTNER_SHARED: 1005 if ((info[epidx].zone == 1006 FJES_ZONING_ZONE_TYPE_NONE) || 1007 (info[epidx].es_status != 1008 FJES_ZONING_STATUS_ENABLE) || 1009 (info[epidx].zone != 1010 info[hw->my_epid].zone)) 1011 set_bit(epidx, &irq_bit); 1012 break; 1013 } 1014 } 1015 1016 hw->ep_shm_info[epidx].es_status = info[epidx].es_status; 1017 hw->ep_shm_info[epidx].zone = info[epidx].zone; I'm not sure how Smatch is able to generate this warning. The array is allocated using the FJES_DEV_REQ_BUF_SIZE(hw->max_epid) macro. It really has a lot of obfuscation layers so I wasn't able to understand it. It seems like this might be a real bug though. I suspect the fix is to change the continue on line 970 to a break and delete lines 1016 and 1017? 1018 1019 break; 1020 } regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch -next] net/mlx5_core: fix an error code
We return success if mlx5e_alloc_sq_db() fails but we should return an error code. Fixes: f62b8bb8f2d3 ('net/mlx5: Extend mlx5_core to support ConnectX-4 Ethernet functionality') Signed-off-by: Dan Carpenter diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c index 7348c51..075e517 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c @@ -525,7 +525,8 @@ static int mlx5e_create_sq(struct mlx5e_channel *c, sq->uar_map = sq->uar.map; sq->bf_buf_size = (1 << MLX5_CAP_GEN(mdev, log_bf_reg_size)) / 2; - if (mlx5e_alloc_sq_db(sq, cpu_to_node(c->cpu))) + err = mlx5e_alloc_sq_db(sq, cpu_to_node(c->cpu)); + if (err) goto err_sq_wq_destroy; sq->txq = netdev_get_tx_queue(priv->netdev, -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch -next] renesas: missing unlock on error path
We need to unlock before returning here. Fixes: a0d2f20650e8 ('Renesas Ethernet AVB PTP clock driver') Signed-off-by: Dan Carpenter diff --git a/drivers/net/ethernet/renesas/ravb_ptp.c b/drivers/net/ethernet/renesas/ravb_ptp.c index 42656da..7a8ce92 100644 --- a/drivers/net/ethernet/renesas/ravb_ptp.c +++ b/drivers/net/ethernet/renesas/ravb_ptp.c @@ -116,8 +116,10 @@ static int ravb_ptp_adjfreq(struct ptp_clock_info *ptp, s32 ppb) priv->ptp.current_addend = addend; gccr = ravb_read(ndev, GCCR); - if (gccr & GCCR_LTI) + if (gccr & GCCR_LTI) { + spin_unlock_irqrestore(&priv->lock, flags); return -EBUSY; + } ravb_write(ndev, addend & GTI_TIV, GTI); ravb_write(ndev, gccr | GCCR_LTI, GCCR); -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch -next] cavium/liquidio: fix some error handling in lio_set_phys_id()
There was a missing assignment so the "if (ret)" on the next line is never true. Fixes: f21fb3ed364b ('Add support of Cavium Liquidio ethernet adapters') Signed-off-by: Dan Carpenter diff --git a/drivers/net/ethernet/cavium/liquidio/lio_ethtool.c b/drivers/net/ethernet/cavium/liquidio/lio_ethtool.c index 160f807..29f3308 100644 --- a/drivers/net/ethernet/cavium/liquidio/lio_ethtool.c +++ b/drivers/net/ethernet/cavium/liquidio/lio_ethtool.c @@ -434,8 +434,9 @@ static int lio_set_phys_id(struct net_device *netdev, if (ret) return ret; - octnet_mdio45_access(lio, 1, LIO68XX_LED_BEACON_ADDR, -&lio->phy_beacon_val); + ret = octnet_mdio45_access(lio, 1, + LIO68XX_LED_BEACON_ADDR, + &lio->phy_beacon_val); if (ret) return ret; -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net-next] hv_netvsc: Add support to set MTU reservation from guest side
On Thu, Jul 02, 2015 at 01:17:35PM -0700, Haiyang Zhang wrote: > When packet encapsulation is in use, the MTU needs to be reduced for > headroom reservation. > The existing code takes the updated MTU value only from the host side. > But vSwitch extensions, such as Open vSwitch, require the flexibility > to change the MTU to different values from within a guest during the > lifecycle of a vNIC, when the encapsulation protocol is changed. The > patch supports this kind of MTU changes. > > Signed-off-by: Haiyang Zhang > Reviewed-by: K. Y. Srinivasan > --- > drivers/net/hyperv/netvsc_drv.c |3 +-- > drivers/net/hyperv/rndis_filter.c |2 +- > 2 files changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c > index 358475e..68e7ece 100644 > --- a/drivers/net/hyperv/netvsc_drv.c > +++ b/drivers/net/hyperv/netvsc_drv.c > @@ -743,8 +743,7 @@ static int netvsc_change_mtu(struct net_device *ndev, int > mtu) > if (nvdev->nvsp_version >= NVSP_PROTOCOL_VERSION_2) > limit = NETVSC_MTU - ETH_HLEN; > > - /* Hyper-V hosts don't support MTU < ETH_DATA_LEN (1500) */ > - if (mtu < ETH_DATA_LEN || mtu > limit) > + if (mtu < 68 || mtu > limit) How did you calculate 68? Avoid magic numbers like this, make it a define. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] net/xen-netback: off by one in BUG_ON() condition
The > should be >=. I also added spaces around the '-' operations so the code is a little more consistent and matches the condition better. Fixes: f53c3fe8dad7 ('xen-netback: Introduce TX grant mapping') Signed-off-by: Dan Carpenter diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c index 880d0d6..7d50711 100644 --- a/drivers/net/xen-netback/netback.c +++ b/drivers/net/xen-netback/netback.c @@ -1566,13 +1566,13 @@ static inline void xenvif_tx_dealloc_action(struct xenvif_queue *queue) smp_rmb(); while (dc != dp) { - BUG_ON(gop - queue->tx_unmap_ops > MAX_PENDING_REQS); + BUG_ON(gop - queue->tx_unmap_ops >= MAX_PENDING_REQS); pending_idx = queue->dealloc_ring[pending_index(dc++)]; - pending_idx_release[gop-queue->tx_unmap_ops] = + pending_idx_release[gop - queue->tx_unmap_ops] = pending_idx; - queue->pages_to_unmap[gop-queue->tx_unmap_ops] = + queue->pages_to_unmap[gop - queue->tx_unmap_ops] = queue->mmap_pages[pending_idx]; gnttab_set_unmap_op(gop, idx_to_kaddr(queue, pending_idx), -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] staging: r8712u: Fix kernel warning for improper call of del_timer_sync()
On Sun, May 24, 2015 at 07:11:40PM -0500, Larry Finger wrote: > On 05/24/2015 02:03 PM, Haggai Eran wrote: > >On 24 May 2015 at 00:16, Larry Finger wrote: > >>The driver is reporting a warning at kernel/time/timer.c:1096 due to calling > >>del_timer_sync() while in interrupt mode. Such warnings are fixed by calling > >>del_timer() instead. > >> > >>Signed-off-by: Larry Finger > >>Cc: Stable > >>Cc: Haggi Eran > > > >Hi, > > > >I haven't been using kernel v4.1 so I haven't seen this warning, but looking > >at the code it seems to originate from the two recent patches to remove > >_cancel_timer and _cancel_timer_ex. I see that there's another patch in lkml > >[1] > >that changes del_timer_sync back to del_timer in more places. Perhaps it > >could prevent other warnings like this in the future. > > > >Regards, > >Haggai > > > >[1] https://lkml.org/lkml/2015/5/15/226 > > Yes, the script kiddies make changes they do not understand and > screw everything up. Unfortunately, I did not catch these in review. > I think I will submit V2 and blast the contributor. Don't blast the contributor... These are special intern patches that dont' go through the normal review process. The intern process is over this year. The lack of normal review introduced a number of bugs this year. I always complain to Greg about it and he says that I should join the intern mailing list if I care so much. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html