[patch -next] qed: potential overflow in qed_cxt_src_t2_alloc()

2016-06-07 Thread Dan Carpenter
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

2016-05-03 Thread Dan Carpenter
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

2016-05-03 Thread Dan Carpenter
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

2016-05-03 Thread Dan Carpenter
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()

2016-05-05 Thread Dan Carpenter
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

2016-05-05 Thread Dan Carpenter
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

2016-05-05 Thread Dan Carpenter
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()

2016-05-05 Thread Dan Carpenter
"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()

2016-05-05 Thread Dan Carpenter
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()

2016-05-10 Thread Dan Carpenter
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

2016-05-13 Thread Dan Carpenter
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

2016-05-17 Thread Dan Carpenter
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

2016-05-17 Thread Dan Carpenter
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

2016-05-17 Thread Dan Carpenter
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()

2016-05-23 Thread Dan Carpenter
"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()

2016-05-25 Thread Dan Carpenter
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

2016-05-27 Thread Dan Carpenter
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()

2016-05-27 Thread Dan Carpenter
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()

2016-02-23 Thread Dan Carpenter
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

2016-03-15 Thread Dan Carpenter
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()

2016-03-15 Thread Dan Carpenter
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

2016-03-16 Thread Dan Carpenter
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

2016-03-21 Thread Dan Carpenter
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

2016-02-27 Thread Dan Carpenter
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

2016-03-02 Thread Dan Carpenter
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

2016-03-02 Thread Dan Carpenter
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

2016-03-02 Thread Dan Carpenter
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

2016-03-08 Thread Dan Carpenter
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'

2016-04-11 Thread Dan Carpenter
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

2016-04-18 Thread Dan Carpenter
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()

2016-04-19 Thread Dan Carpenter
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

2016-04-27 Thread Dan Carpenter
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

2015-12-08 Thread Dan Carpenter
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()

2015-12-09 Thread Dan Carpenter
"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

2015-12-09 Thread Dan Carpenter
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

2015-12-09 Thread Dan Carpenter
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

2015-12-14 Thread Dan Carpenter
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

2015-12-15 Thread Dan Carpenter
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

2015-12-15 Thread Dan Carpenter
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

2015-12-15 Thread Dan Carpenter
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()

2015-12-15 Thread Dan Carpenter
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

2015-12-15 Thread Dan Carpenter
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

2015-12-15 Thread Dan Carpenter
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

2015-12-15 Thread Dan Carpenter
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

2015-12-15 Thread Dan Carpenter
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()

2016-06-17 Thread Dan Carpenter
"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

2016-06-18 Thread Dan Carpenter
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

2016-06-18 Thread Dan Carpenter
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

2016-06-18 Thread Dan Carpenter
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()

2016-06-18 Thread Dan Carpenter
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()

2016-06-27 Thread Dan Carpenter
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()

2016-06-29 Thread Dan Carpenter
"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

2015-12-24 Thread Dan Carpenter
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()

2016-01-04 Thread Dan Carpenter
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()

2016-01-04 Thread Dan Carpenter
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

2016-01-04 Thread Dan Carpenter
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()

2016-01-04 Thread Dan Carpenter
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()

2016-01-04 Thread Dan Carpenter
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

2016-01-05 Thread Dan Carpenter
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

2016-01-06 Thread Dan Carpenter
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

2016-01-06 Thread Dan Carpenter
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

2016-01-06 Thread Dan Carpenter
"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

2016-01-06 Thread Dan Carpenter
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

2016-01-06 Thread Dan Carpenter
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()

2015-10-19 Thread Dan Carpenter
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

2015-10-19 Thread Dan Carpenter
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

2015-11-04 Thread Dan Carpenter
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

2015-11-04 Thread Dan Carpenter
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()

2015-11-05 Thread Dan Carpenter
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

2015-11-13 Thread Dan Carpenter
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

2015-11-13 Thread Dan Carpenter
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

2015-11-21 Thread Dan Carpenter
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 *

2015-11-24 Thread Dan Carpenter
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()

2015-08-08 Thread Dan Carpenter
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

2015-08-08 Thread Dan Carpenter
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()

2015-08-10 Thread Dan Carpenter
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()

2015-08-12 Thread Dan Carpenter
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()

2015-08-13 Thread Dan Carpenter
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

2015-08-14 Thread Dan Carpenter
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

2015-08-18 Thread Dan Carpenter
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

2015-08-18 Thread Dan Carpenter
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

2015-10-03 Thread Dan Carpenter
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

2015-07-22 Thread Dan Carpenter
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

2015-07-23 Thread Dan Carpenter
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

2015-07-23 Thread Dan Carpenter
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

2015-07-24 Thread Dan Carpenter
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()

2015-07-27 Thread Dan Carpenter
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()

2015-07-27 Thread Dan Carpenter
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

2015-07-27 Thread Dan Carpenter
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

2015-07-30 Thread Dan Carpenter
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()

2015-08-01 Thread Dan Carpenter
"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

2015-08-04 Thread Dan Carpenter
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()

2015-08-04 Thread Dan Carpenter
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

2015-09-13 Thread Dan Carpenter
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

2015-06-11 Thread Dan Carpenter
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

2015-06-24 Thread Dan Carpenter
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()

2015-06-24 Thread Dan Carpenter
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

2015-07-03 Thread Dan Carpenter
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

2015-07-11 Thread Dan Carpenter
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()

2015-05-25 Thread Dan Carpenter
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


  1   2   3   4   5   6   >