[PATCH v3] {net, IB}/mlx5: Use 'kvfree()' for memory allocated by 'kvzalloc()'

[Christophe JAILLET]

When 'kvzalloc()' is used to allocate memory, 'kvfree()' must be used to
free it.

Fixes: 1cbe6fc86ccfe ("IB/mlx5: Add support for CQE compressing")
Fixes: fed9ce22bf8ae ("net/mlx5: E-Switch, Add API to create vport rx rules")
Fixes: 9efa75254593d ("net/mlx5_core: Introduce access functions to query vport 
RoCE fields")
Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
v1 -> v2: More places to update have been added to the patch
v2 -> v3: Add Fixes tag

3 patches with one Fixes tag each should probably be better, but honestly, I 
won't send a v4.
Fill free to split it if needed.
---
 drivers/infiniband/hw/mlx5/cq.c| 2 +-
 drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c | 2 +-
 drivers/net/ethernet/mellanox/mlx5/core/vport.c| 6 +++---
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/hw/mlx5/cq.c b/drivers/infiniband/hw/mlx5/cq.c
index 77d257ec899b..6d52ea03574e 100644
--- a/drivers/infiniband/hw/mlx5/cq.c
+++ b/drivers/infiniband/hw/mlx5/cq.c
@@ -849,7 +849,7 @@ static int create_cq_user(struct mlx5_ib_dev *dev, struct 
ib_udata *udata,
return 0;
 
 err_cqb:
-   kfree(*cqb);
+   kvfree(*cqb);
 
 err_db:
mlx5_ib_db_unmap_user(to_mucontext(context), >db);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c 
b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
index 35e256eb2f6e..b123f8a52ad8 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
@@ -663,7 +663,7 @@ static int esw_create_vport_rx_group(struct mlx5_eswitch 
*esw)
 
esw->offloads.vport_rx_group = g;
 out:
-   kfree(flow_group_in);
+   kvfree(flow_group_in);
return err;
 }
 
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/vport.c 
b/drivers/net/ethernet/mellanox/mlx5/core/vport.c
index 177e076b8d17..719cecb182c6 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/vport.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/vport.c
@@ -511,7 +511,7 @@ int mlx5_query_nic_vport_system_image_guid(struct 
mlx5_core_dev *mdev,
*system_image_guid = MLX5_GET64(query_nic_vport_context_out, out,
nic_vport_context.system_image_guid);
 
-   kfree(out);
+   kvfree(out);
 
return 0;
 }
@@ -531,7 +531,7 @@ int mlx5_query_nic_vport_node_guid(struct mlx5_core_dev 
*mdev, u64 *node_guid)
*node_guid = MLX5_GET64(query_nic_vport_context_out, out,
nic_vport_context.node_guid);
 
-   kfree(out);
+   kvfree(out);
 
return 0;
 }
@@ -587,7 +587,7 @@ int mlx5_query_nic_vport_qkey_viol_cntr(struct 
mlx5_core_dev *mdev,
*qkey_viol_cntr = MLX5_GET(query_nic_vport_context_out, out,
   nic_vport_context.qkey_violation_counter);
 
-   kfree(out);
+   kvfree(out);
 
return 0;
 }
-- 
2.17.0

Re: [PATCH] net/mlx5: Use 'kvfree()' for memory allocated by 'kvzalloc()'

[Christophe Jaillet]

Le 14/05/2018 à 20:56, David Miller a écrit :

From: Christophe JAILLET <christophe.jail...@wanadoo.fr>
Date: Sat, 12 May 2018 19:09:25 +0200


'out' is allocated with 'kvzalloc()'. 'kvfree()' must be used to free it.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>


Saeed, I assume I will see this in one of your forthcoming pull
requests.

Thanks.



I've send a v2 with additional 'kvfree()' fixes.

CJ

[PATCH v2] {net, IB}/mlx5: Use 'kvfree()' for memory allocated by 'kvzalloc()'

[Christophe JAILLET]

When 'kvzalloc()' is used to allocate memory, 'kvfree()' must be used to
free it.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
v1 -> v2: More places to update have been added to the patch
---
 drivers/infiniband/hw/mlx5/cq.c| 2 +-
 drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c | 2 +-
 drivers/net/ethernet/mellanox/mlx5/core/vport.c| 6 +++---
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/hw/mlx5/cq.c b/drivers/infiniband/hw/mlx5/cq.c
index 77d257ec899b..6d52ea03574e 100644
--- a/drivers/infiniband/hw/mlx5/cq.c
+++ b/drivers/infiniband/hw/mlx5/cq.c
@@ -849,7 +849,7 @@ static int create_cq_user(struct mlx5_ib_dev *dev, struct 
ib_udata *udata,
return 0;
 
 err_cqb:
-   kfree(*cqb);
+   kvfree(*cqb);
 
 err_db:
mlx5_ib_db_unmap_user(to_mucontext(context), >db);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c 
b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
index 35e256eb2f6e..b123f8a52ad8 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
@@ -663,7 +663,7 @@ static int esw_create_vport_rx_group(struct mlx5_eswitch 
*esw)
 
esw->offloads.vport_rx_group = g;
 out:
-   kfree(flow_group_in);
+   kvfree(flow_group_in);
return err;
 }
 
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/vport.c 
b/drivers/net/ethernet/mellanox/mlx5/core/vport.c
index 177e076b8d17..719cecb182c6 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/vport.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/vport.c
@@ -511,7 +511,7 @@ int mlx5_query_nic_vport_system_image_guid(struct 
mlx5_core_dev *mdev,
*system_image_guid = MLX5_GET64(query_nic_vport_context_out, out,
nic_vport_context.system_image_guid);
 
-   kfree(out);
+   kvfree(out);
 
return 0;
 }
@@ -531,7 +531,7 @@ int mlx5_query_nic_vport_node_guid(struct mlx5_core_dev 
*mdev, u64 *node_guid)
*node_guid = MLX5_GET64(query_nic_vport_context_out, out,
nic_vport_context.node_guid);
 
-   kfree(out);
+   kvfree(out);
 
return 0;
 }
@@ -587,7 +587,7 @@ int mlx5_query_nic_vport_qkey_viol_cntr(struct 
mlx5_core_dev *mdev,
*qkey_viol_cntr = MLX5_GET(query_nic_vport_context_out, out,
   nic_vport_context.qkey_violation_counter);
 
-   kfree(out);
+   kvfree(out);
 
return 0;
 }
-- 
2.17.0

[PATCH] net/mlx5: Use 'kvfree()' for memory allocated by 'kvzalloc()'

[Christophe JAILLET]

'out' is allocated with 'kvzalloc()'. 'kvfree()' must be used to free it.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/ethernet/mellanox/mlx5/core/vport.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/vport.c 
b/drivers/net/ethernet/mellanox/mlx5/core/vport.c
index 177e076b8d17..49968a4db758 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/vport.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/vport.c
@@ -511,7 +511,7 @@ int mlx5_query_nic_vport_system_image_guid(struct 
mlx5_core_dev *mdev,
*system_image_guid = MLX5_GET64(query_nic_vport_context_out, out,
nic_vport_context.system_image_guid);
 
-   kfree(out);
+   kvfree(out);
 
return 0;
 }
-- 
2.17.0

Re: [PATCH] net: aquantia: Fix an error handling path in 'aq_pci_probe()'

[Christophe JAILLET]

Le 08/05/2018 à 09:19, Igor Russkikh a écrit :


This was just submitted yesterday and is already accepted in netdev by David:

http://patchwork.ozlabs.org/patch/909746/

Thanks!

BR, Igor


Patch timing is sometimes surprising!

Not sure at all if it can be an issue, but I also noted that the order 
of 'pci_release_regions()' and 'free_netdev()' is in reverse

order in the 'aq_pci_remove()' function.
I don't know if done on purpose and/or needed, so I've left it as-is.

CJ

Re: [PATCH] net/mlx4_en: Fix an error handling path in 'mlx4_en_init_netdev()'

[Christophe JAILLET]

Le 10/05/2018 à 15:38, Yuval Shaia a écrit :

On Thu, May 10, 2018 at 09:02:26AM +0200, Christophe JAILLET wrote:

If an error occurs, 'mlx4_en_destroy_netdev()' is called.
It then calls 'mlx4_en_free_resources()' which does the needed resources
cleanup.

So, doing some explicit kfree in the error handling path would lead to
some double kfree.

Patch make sense but what's bothering me is that mlx4_en_free_resources
loops on the entire array, assuming !priv->tx_ring[t] means entry is
allocated but the existing code does not assume that, see [1]. So i looked
to see where tx_ring array is zeroed and didn't find it.

Am i missing something here.
My understanding is that the array is zeoed at line 3289, when the whole 
priv struct is memset(0)'ed (also done in alloc_etherdev_mqs but leaving 
an explicit memset help to remind that the struct is zeroed)
If speed matters here (and I don't think so), the memset could be saved 
(the mlx4_en_priv struct is quite big after all), but at least a comment 
should remind that it is initialized within alloc_etherdev_mqs.


CJ



Simplify code to avoid such a case.

Fixes: 67f8b1dcb9ee ("net/mlx4_en: Refactor the XDP forwarding rings scheme")
Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
  drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 8 +---
  1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c 
b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index e0adac4a9a19..9670b33fc9b1 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -3324,12 +3324,11 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int 
port,
   MAX_TX_RINGS, GFP_KERNEL);
if (!priv->tx_ring[t]) {
err = -ENOMEM;
-   goto err_free_tx;
+   goto out;
}
priv->tx_cq[t] = kzalloc(sizeof(struct mlx4_en_cq *) *
 MAX_TX_RINGS, GFP_KERNEL);
if (!priv->tx_cq[t]) {
-   kfree(priv->tx_ring[t]);
err = -ENOMEM;
goto out;
}
@@ -3582,11 +3581,6 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int 
port,
  
  	return 0;
  
-err_free_tx:

-   while (t--) {

[1]


-   kfree(priv->tx_ring[t]);
-   kfree(priv->tx_cq[t]);
-   }
  out:
mlx4_en_destroy_netdev(dev);
return err;
--
2.17.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] mlxsw: core: Fix an error handling path in 'mlxsw_core_bus_device_register()'

[Christophe JAILLET]

Resources are not freed in the reverse order of the allocation.
Labels are also mixed-up.

Fix it and reorder code and labels in the error handling path of
'mlxsw_core_bus_device_register()'

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
Please review carefully. This patch is proposed because it triggers one of
my coccinelle scripts. I'm not 100% sure if correct.

The script tries to spot wrongly ordered error handling path. It is:
@@
identifier l1, l2;
@@

   if (...) {
  ...
* goto l1;
   }
   ...
   if (...) {
  ...
* goto l2;
   }
   ...
*l1:
   ...
*l2:
   ...
---
 drivers/net/ethernet/mellanox/mlxsw/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c 
b/drivers/net/ethernet/mellanox/mlxsw/core.c
index 93ea56620a24..e13ac3b8dff7 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -1100,11 +1100,11 @@ int mlxsw_core_bus_device_register(const struct 
mlxsw_bus_info *mlxsw_bus_info,
 err_alloc_lag_mapping:
mlxsw_ports_fini(mlxsw_core);
 err_ports_init:
-   mlxsw_bus->fini(bus_priv);
-err_bus_init:
if (!reload)
devlink_resources_unregister(devlink, NULL);
 err_register_resources:
+   mlxsw_bus->fini(bus_priv);
+err_bus_init:
if (!reload)
devlink_free(devlink);
 err_devlink_alloc:
-- 
2.17.0

[PATCH v2] net/mlx4_en: Fix an error handling path in 'mlx4_en_init_netdev()'

[Christophe JAILLET]

If an error occurs, 'mlx4_en_destroy_netdev()' is called.
It then calls 'mlx4_en_free_resources()' which does the needed resources
cleanup.

So, doing some explicit kfree in the error handling path would lead to
some double kfree.

Simplify code to avoid such a case.

Fixes: 67f8b1dcb9ee ("net/mlx4_en: Refactor the XDP forwarding rings scheme")
Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
v1 -> v2 : rewrite the fix as explained by Tariq Toukan
   (this 2nd version may have been posted twice, once without the
   v2 tag. PLease ignore the first one)
---

 drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c 
b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index e0adac4a9a19..9670b33fc9b1 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -3324,12 +3324,11 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int 
port,
   MAX_TX_RINGS, GFP_KERNEL);
if (!priv->tx_ring[t]) {
err = -ENOMEM;
-   goto err_free_tx;
+   goto out;
}
priv->tx_cq[t] = kzalloc(sizeof(struct mlx4_en_cq *) *
 MAX_TX_RINGS, GFP_KERNEL);
if (!priv->tx_cq[t]) {
-   kfree(priv->tx_ring[t]);
err = -ENOMEM;
goto out;
}
@@ -3582,11 +3581,6 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int 
port,
 
return 0;
 
-err_free_tx:
-   while (t--) {
-   kfree(priv->tx_ring[t]);
-   kfree(priv->tx_cq[t]);
-   }
 out:
mlx4_en_destroy_netdev(dev);
return err;
-- 
2.17.0

[PATCH] net/mlx4_en: Fix an error handling path in 'mlx4_en_init_netdev()'

[Christophe JAILLET]

If an error occurs, 'mlx4_en_destroy_netdev()' is called.
It then calls 'mlx4_en_free_resources()' which does the needed resources
cleanup.

So, doing some explicit kfree in the error handling path would lead to
some double kfree.

Simplify code to avoid such a case.

Fixes: 67f8b1dcb9ee ("net/mlx4_en: Refactor the XDP forwarding rings scheme")
Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c 
b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index e0adac4a9a19..9670b33fc9b1 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -3324,12 +3324,11 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int 
port,
   MAX_TX_RINGS, GFP_KERNEL);
if (!priv->tx_ring[t]) {
err = -ENOMEM;
-   goto err_free_tx;
+   goto out;
}
priv->tx_cq[t] = kzalloc(sizeof(struct mlx4_en_cq *) *
 MAX_TX_RINGS, GFP_KERNEL);
if (!priv->tx_cq[t]) {
-   kfree(priv->tx_ring[t]);
err = -ENOMEM;
goto out;
}
@@ -3582,11 +3581,6 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int 
port,
 
return 0;
 
-err_free_tx:
-   while (t--) {
-   kfree(priv->tx_ring[t]);
-   kfree(priv->tx_cq[t]);
-   }
 out:
mlx4_en_destroy_netdev(dev);
return err;
-- 
2.17.0

[PATCH] net/mlx4_en: Fix an error handling path in 'mlx4_en_init_netdev()'

[Christophe JAILLET]

If the 2nd memory allocation of the loop fails, we must undo the
memory allocation done so far.

Fixes: 67f8b1dcb9ee ("net/mlx4_en: Refactor the XDP forwarding rings scheme")
Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c 
b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index e0adac4a9a19..bf078244e467 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -3331,7 +3331,7 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int 
port,
if (!priv->tx_cq[t]) {
kfree(priv->tx_ring[t]);
err = -ENOMEM;
-   goto out;
+   goto err_free_tx;
}
}
priv->rx_ring_num = prof->rx_ring_num;
-- 
2.17.0

[PATCH] net: aquantia: Fix an error handling path in 'aq_pci_probe()'

[Christophe JAILLET]

The position of 2 labels should be swapped in order to release resources
in the correct order and avoid leaks.

Fixes: 23ee07ad3c2f ("net: aquantia: Cleanup pci functions module")
Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
The order of 'pci_release_regions()' and 'free_netdev()' is in reverse
order in the 'aq_pci_remove()' function.
I don't know if done on purpose and/or needed, so I've left it as-is.
---
 drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c 
b/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c
index ecc6306f940f..b7f6b5a68b33 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c
@@ -298,9 +298,9 @@ static int aq_pci_probe(struct pci_dev *pdev,
kfree(self->aq_hw);
 err_ioremap:
free_netdev(ndev);
-err_pci_func:
-   pci_release_regions(pdev);
 err_ndev:
+   pci_release_regions(pdev);
+err_pci_func:
pci_disable_device(pdev);
return err;
 }
-- 
2.17.0

[PATCH v2] net: ethernet: arc: Fix a potential memory leak if an optional regulator is deferred

[Christophe JAILLET]

If the optional regulator is deferred, we must release some resources.
They will be re-allocated when the probe function will be called again.

Fixes: 6eacf31139bf ("ethernet: arc: Add support for Rockchip SoC layer device 
tree bindings")
Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
v2: v1 did not compile because of an erroneous variable name. s/ret/err/
---
 drivers/net/ethernet/arc/emac_rockchip.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/arc/emac_rockchip.c 
b/drivers/net/ethernet/arc/emac_rockchip.c
index 16f9bee992fe..8ee9dfd0e363 100644
--- a/drivers/net/ethernet/arc/emac_rockchip.c
+++ b/drivers/net/ethernet/arc/emac_rockchip.c
@@ -169,8 +169,10 @@ static int emac_rockchip_probe(struct platform_device 
*pdev)
/* Optional regulator for PHY */
priv->regulator = devm_regulator_get_optional(dev, "phy");
if (IS_ERR(priv->regulator)) {
-   if (PTR_ERR(priv->regulator) == -EPROBE_DEFER)
-   return -EPROBE_DEFER;
+   if (PTR_ERR(priv->regulator) == -EPROBE_DEFER) {
+   err = -EPROBE_DEFER;
+   goto out_clk_disable;
+   }
dev_err(dev, "no regulator found\n");
priv->regulator = NULL;
}
-- 
2.14.1

Re: [PATCH] net: ethernet: arc: Fix a potential memory leak if an optional regulator is deferred

[Christophe JAILLET]

Le 16/03/2018 à 20:27, David Miller a écrit :

From: Christophe JAILLET <christophe.jail...@wanadoo.fr>
Date: Wed, 14 Mar 2018 22:09:34 +0100


diff --git a/drivers/net/ethernet/arc/emac_rockchip.c 
b/drivers/net/ethernet/arc/emac_rockchip.c
index 16f9bee992fe..8ee9dfd0e363 100644
--- a/drivers/net/ethernet/arc/emac_rockchip.c
+++ b/drivers/net/ethernet/arc/emac_rockchip.c
@@ -169,8 +169,10 @@ static int emac_rockchip_probe(struct platform_device 
*pdev)
/* Optional regulator for PHY */
priv->regulator = devm_regulator_get_optional(dev, "phy");
if (IS_ERR(priv->regulator)) {
-   if (PTR_ERR(priv->regulator) == -EPROBE_DEFER)
-   return -EPROBE_DEFER;
+   if (PTR_ERR(priv->regulator) == -EPROBE_DEFER) {
+   ret = -EPROBE_DEFER;
+   goto out_clk_disable;
+   }

Please build test your changes.

There is no 'ret' variable in this function, perhaps you meant 'err'.


Yes, obviously, this is 'err'.

I apologize. I usually build-test all my patches before submission.
This one seems to have gone out of my normal process.

Can you fix it yourself or do you want me to re-submit?

CJ

[PATCH] net: ethernet: arc: Fix a potential memory leak if an optional regulator is deferred

[Christophe JAILLET]

If the optional regulator is deferrred, we must release some resources.
They will be re-allocated when the probe function will be called again.

Fixes: 6eacf31139bf ("ethernet: arc: Add support for Rockchip SoC layer device 
tree bindings")
Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/ethernet/arc/emac_rockchip.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/arc/emac_rockchip.c 
b/drivers/net/ethernet/arc/emac_rockchip.c
index 16f9bee992fe..8ee9dfd0e363 100644
--- a/drivers/net/ethernet/arc/emac_rockchip.c
+++ b/drivers/net/ethernet/arc/emac_rockchip.c
@@ -169,8 +169,10 @@ static int emac_rockchip_probe(struct platform_device 
*pdev)
/* Optional regulator for PHY */
priv->regulator = devm_regulator_get_optional(dev, "phy");
if (IS_ERR(priv->regulator)) {
-   if (PTR_ERR(priv->regulator) == -EPROBE_DEFER)
-   return -EPROBE_DEFER;
+   if (PTR_ERR(priv->regulator) == -EPROBE_DEFER) {
+   ret = -EPROBE_DEFER;
+   goto out_clk_disable;
+   }
dev_err(dev, "no regulator found\n");
priv->regulator = NULL;
}
-- 
2.14.1

Re: [PATCH] net/mlx4_en: Fix a memory leak in case of error in 'mlx4_en_init_netdev()'

[Christophe Jaillet]

Le 12/03/2018 à 09:42, Tariq Toukan a écrit :
>
>
> On 12/03/2018 12:45 AM, Christophe JAILLET wrote:
>> If 'kzalloc' fails, we must free some memory before returning.
>>
>> Fixes: 67f8b1dcb9ee ("net/mlx4_en: Refactor the XDP forwarding rings
>> scheme")
>> Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
>> ---
>>   drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
>> b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
>> index 8fc51bc29003..f9db018e858f 100644
>> --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
>> +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
>> @@ -3327,7 +3327,7 @@ int mlx4_en_init_netdev(struct mlx4_en_dev
>> *mdev, int port,
>>   if (!priv->tx_cq[t]) {
>>   kfree(priv->tx_ring[t]);
>>   err = -ENOMEM;
>> -goto out;
>> +goto err_free_tx;
>>   }
>>   }
>>   priv->rx_ring_num = prof->rx_ring_num;
>>
>
> Hi Christophe, thanks for spotting this.
>
> However, I think these err_free_tx label and loop are redundant.
> Both tx_ring/tx_cq flows should just goto out, as resources are freed
> later in mlx4_en_destroy_netdev() -> mlx4_en_free_resources().
>

Hi,

I do not agree with you and I think that the patch is relevant.

If 'mlx4_en_init_netdev' fails, the only caller, 'mlx4_en_activate()', 
will set:

mdev->pndev[i] = NULL
(see 
https://elixir.bootlin.com/linux/v4.16-rc5/source/drivers/net/ethernet/mellanox/mlx4/en_main.c#L254)


and 'mlx4_en_destroy_netdev()' is not called in this case.
(see 
https://elixir.bootlin.com/linux/v4.16-rc5/source/drivers/net/ethernet/mellanox/mlx4/en_main.c#L232)


My understanding is that 'mlx4_en_destroy_netdev()' will free resources 
in the normal case but that resources should be freed at allocation time 
if it does not fully succeed.


Best regards,
CJ

---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel 
antivirus Avast.
https://www.avast.com/antivirus

[PATCH] net/mlx4_en: Fix a memory leak in case of error in 'mlx4_en_init_netdev()'

[Christophe JAILLET]

If 'kzalloc' fails, we must free some memory before returning.

Fixes: 67f8b1dcb9ee ("net/mlx4_en: Refactor the XDP forwarding rings scheme")
Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c 
b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index 8fc51bc29003..f9db018e858f 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -3327,7 +3327,7 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int 
port,
if (!priv->tx_cq[t]) {
kfree(priv->tx_ring[t]);
err = -ENOMEM;
-   goto out;
+   goto err_free_tx;
}
}
priv->rx_ring_num = prof->rx_ring_num;
-- 
2.14.1


---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel 
antivirus Avast.
https://www.avast.com/antivirus

[PATCH] cxgb4: Fix error handling path in 'init_one()'

[Christophe JAILLET]

Commit baf5086840ab1 ("cxgb4: restructure VF mgmt code") has reordered
some code but an error handling label has not been updated accordingly.
So fix it and free 'adapter' if 't4_wait_dev_ready()' fails.

Fixes: baf5086840ab1 ("cxgb4: restructure VF mgmt code")
Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c 
b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index 725643b4a8ab..79bffaa3c1af 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -5125,7 +5125,7 @@ static int init_one(struct pci_dev *pdev, const struct 
pci_device_id *ent)
adapter->regs = regs;
err = t4_wait_dev_ready(regs);
if (err < 0)
-   goto out_unmap_bar0;
+   goto out_free_adapter;
 
/* We control everything through one PF */
whoami = readl(regs + PL_WHOAMI_A);
-- 
2.14.1


---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel 
antivirus Avast.
https://www.avast.com/antivirus

[PATCH] igb: Fix a test with HWTSTAMP_TX_ON

[Christophe JAILLET]

'HWTSTAMP_TX_ON' should be handled as a value, not as a bit mask.
The modified code should behave the same, because HWTSTAMP_TX_ON is 1
and no other possible values of 'tx_type' would match the test.
However, this is more future-proof, should other values be allowed one day.

See 'struct hwtstamp_config' in 'include/uapi/linux/net_tstamp.h'

This fixes a warning reported by smatch:
   igb_xmit_frame_ring() warn: bit shifter 'HWTSTAMP_TX_ON' used for logical '&'

Fixes: 26bd4e2db06be ("igb: protect TX timestamping from API misuse")
Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/ethernet/intel/igb/igb_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c 
b/drivers/net/ethernet/intel/igb/igb_main.c
index c208753ff5b7..e945d1f7c7fe 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -5727,7 +5727,7 @@ netdev_tx_t igb_xmit_frame_ring(struct sk_buff *skb,
if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)) {
struct igb_adapter *adapter = netdev_priv(tx_ring->netdev);
 
-   if (adapter->tstamp_config.tx_type & HWTSTAMP_TX_ON &&
+   if (adapter->tstamp_config.tx_type == HWTSTAMP_TX_ON &&
!test_and_set_bit_lock(__IGB_PTP_TX_IN_PROGRESS,
   >state)) {
skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
-- 
2.14.1


---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel 
antivirus Avast.
https://www.avast.com/antivirus

[PATCH] mdio-sun4i: Fix a memory leak

[Christophe JAILLET]

If the probing of the regulator is deferred, the memory allocated by
'mdiobus_alloc_size()' will be leaking.
It should be freed before the next call to 'sun4i_mdio_probe()' which will
reallocate it.

Fixes: 4bdcb1dd9feb ("net: Add MDIO bus driver for the Allwinner EMAC")
Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/phy/mdio-sun4i.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/mdio-sun4i.c b/drivers/net/phy/mdio-sun4i.c
index 135296508a7e..6425ce04d3f9 100644
--- a/drivers/net/phy/mdio-sun4i.c
+++ b/drivers/net/phy/mdio-sun4i.c
@@ -118,8 +118,10 @@ static int sun4i_mdio_probe(struct platform_device *pdev)
 
data->regulator = devm_regulator_get(>dev, "phy");
if (IS_ERR(data->regulator)) {
-   if (PTR_ERR(data->regulator) == -EPROBE_DEFER)
-   return -EPROBE_DEFER;
+   if (PTR_ERR(data->regulator) == -EPROBE_DEFER) {
+   ret = -EPROBE_DEFER;
+   goto err_out_free_mdiobus;
+   }
 
dev_info(>dev, "no regulator found\n");
data->regulator = NULL;
-- 
2.14.1

[PATCH] bnxt_en: Fix an error handling path in 'bnxt_get_module_eeprom()'

[Christophe JAILLET]

Error code returned by 'bnxt_read_sfp_module_eeprom_info()' is handled a
few lines above when reading the A0 portion of the EEPROM.
The same should be done when reading the A2 portion of the EEPROM.

In order to correctly propagate an error, update 'rc' in this 2nd call as
well, otherwise 0 (success) is returned.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
Un-tested.

If not testing the result of the 2nd call was done on purpose, it should be
documented.
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c 
b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index 7ce1d4b7e67d..b13ce5ebde8d 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -2136,8 +2136,8 @@ static int bnxt_get_module_eeprom(struct net_device *dev,
/* Read A2 portion of the EEPROM */
if (length) {
start -= ETH_MODULE_SFF_8436_LEN;
-   bnxt_read_sfp_module_eeprom_info(bp, I2C_DEV_ADDR_A2, 1, start,
-length, data);
+   rc = bnxt_read_sfp_module_eeprom_info(bp, I2C_DEV_ADDR_A2, 1,
+ start, length, data);
}
return rc;
 }
-- 
2.14.1

[PATCH] net: vxge: Fix some indentation issues

[Christophe JAILLET]

Some statements are not enough or too much indented.
Fix it to improve readalbility.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/ethernet/neterion/vxge/vxge-main.c | 37 +-
 1 file changed, 18 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/neterion/vxge/vxge-main.c 
b/drivers/net/ethernet/neterion/vxge/vxge-main.c
index fe7e0e1dd01d..b2299f2b2155 100644
--- a/drivers/net/ethernet/neterion/vxge/vxge-main.c
+++ b/drivers/net/ethernet/neterion/vxge/vxge-main.c
@@ -1530,7 +1530,7 @@ static int vxge_reset_vpath(struct vxgedev *vdev, int 
vp_id)
vxge_debug_init(VXGE_ERR,
"vxge_hw_vpath_reset failed for"
"vpath:%d", vp_id);
-   return status;
+   return status;
}
} else
return VXGE_HW_FAIL;
@@ -1950,19 +1950,19 @@ static enum vxge_hw_status vxge_rth_configure(struct 
vxgedev *vdev)
 * for all VPATHs. The h/w only uses the lowest numbered VPATH
 * when steering frames.
 */
-for (index = 0; index < vdev->no_of_vpath; index++) {
+   for (index = 0; index < vdev->no_of_vpath; index++) {
status = vxge_hw_vpath_rts_rth_set(
vdev->vpaths[index].handle,
vdev->config.rth_algorithm,
_types,
vdev->config.rth_bkt_sz);
-if (status != VXGE_HW_OK) {
+   if (status != VXGE_HW_OK) {
vxge_debug_init(VXGE_ERR,
"RTH configuration failed for vpath:%d",
vdev->vpaths[index].device_id);
return status;
-}
-}
+   }
+   }
 
return status;
 }
@@ -1991,7 +1991,7 @@ static enum vxge_hw_status vxge_reset_all_vpaths(struct 
vxgedev *vdev)
vxge_debug_init(VXGE_ERR,
"vxge_hw_vpath_reset failed for "
"vpath:%d", i);
-   return status;
+   return status;
}
}
}
@@ -2474,32 +2474,31 @@ static int vxge_add_isr(struct vxgedev *vdev)
switch (msix_idx) {
case 0:
snprintf(vdev->desc[intr_cnt], VXGE_INTR_STRLEN,
-   "%s:vxge:MSI-X %d - Tx - fn:%d vpath:%d",
+   "%s:vxge:MSI-X %d - Tx - fn:%d 
vpath:%d",
vdev->ndev->name,
vdev->entries[intr_cnt].entry,
pci_fun, vp_idx);
ret = request_irq(
-   vdev->entries[intr_cnt].vector,
+   vdev->entries[intr_cnt].vector,
vxge_tx_msix_handle, 0,
vdev->desc[intr_cnt],
>vpaths[vp_idx].fifo);
-   vdev->vxge_entries[intr_cnt].arg =
+   vdev->vxge_entries[intr_cnt].arg =
>vpaths[vp_idx].fifo;
irq_req = 1;
break;
case 1:
snprintf(vdev->desc[intr_cnt], VXGE_INTR_STRLEN,
-   "%s:vxge:MSI-X %d - Rx - fn:%d vpath:%d",
+   "%s:vxge:MSI-X %d - Rx - fn:%d 
vpath:%d",
vdev->ndev->name,
vdev->entries[intr_cnt].entry,
pci_fun, vp_idx);
ret = request_irq(
-   vdev->entries[intr_cnt].vector,
-   vxge_rx_msix_napi_handle,
-   0,
+   vdev->entries[intr_cnt].vector,
+   vxge_rx_msix_napi_handle, 0,
vdev->desc[intr_cnt],
>vpaths[vp_idx].ring);
-   vdev->vxge_entries[intr_cnt].arg =
+   vdev->vxge_entries[intr_cnt].arg =
 

[PATCH 1/4] fsl/fman: Remove a useless call to 'dev_set_drvdata()'

[Christophe JAILLET]

Commit c6e26ea8c893 ("dpaa_eth: change device used") has removed usage of
'dev_set_drvdata()' in the 'mac_probe() function.

This call should also be axed.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/ethernet/freescale/fman/mac.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/fman/mac.c 
b/drivers/net/ethernet/freescale/fman/mac.c
index 1d6da1ea7bfb..c27667a005f7 100644
--- a/drivers/net/ethernet/freescale/fman/mac.c
+++ b/drivers/net/ethernet/freescale/fman/mac.c
@@ -713,7 +713,6 @@ static int mac_probe(struct platform_device *_of_dev)
__devm_release_region(dev, fman_get_mem_region(priv->fman),
  res.start, res.end + 1 - res.start);
devm_kfree(dev, mac_dev);
-   dev_set_drvdata(dev, NULL);
return -ENODEV;
}
 
-- 
2.14.1

[PATCH 3/4] fsl/fman: Add a missing 'of_node_put()' call in an error handling path

[Christophe JAILLET]

If 'of_phy_find_device()' fails, we must undo the previous 'of_node_get()'
call, as done the the following error handling code.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/ethernet/freescale/fman/mac.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/freescale/fman/mac.c 
b/drivers/net/ethernet/freescale/fman/mac.c
index ca12e28129ed..86c1e69f44d6 100644
--- a/drivers/net/ethernet/freescale/fman/mac.c
+++ b/drivers/net/ethernet/freescale/fman/mac.c
@@ -821,6 +821,7 @@ static int mac_probe(struct platform_device *_of_dev)
phy = of_phy_find_device(mac_dev->phy_node);
if (!phy) {
err = -EINVAL;
+   of_node_put(mac_dev->phy_node);
goto _return_of_get_parent;
}
 
-- 
2.14.1

[PATCH 4/4] fsl/fman: Remove a useless 'dev_err()' call

[Christophe JAILLET]

Memory allocation functions already display some informaton in case of
memory allocation failure. There is no need to add an extra 'dev_err' here.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/ethernet/freescale/fman/mac.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/fman/mac.c 
b/drivers/net/ethernet/freescale/fman/mac.c
index 86c1e69f44d6..88c0a0636b44 100644
--- a/drivers/net/ethernet/freescale/fman/mac.c
+++ b/drivers/net/ethernet/freescale/fman/mac.c
@@ -615,7 +615,6 @@ static int mac_probe(struct platform_device *_of_dev)
mac_dev = devm_kzalloc(dev, sizeof(*mac_dev), GFP_KERNEL);
if (!mac_dev) {
err = -ENOMEM;
-   dev_err(dev, "devm_kzalloc() = %d\n", err);
goto _return;
}
priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
-- 
2.14.1

[PATCH 2/4] fsl/fman: Remove some useless code

[Christophe JAILLET]

There is no need to release explicitly some devm_ allocated resources.
If the 'mac_probe()' probe function fails, they will be released
automatically, as already done in the other error handling paths of
this function.

Also goto '_return_of_get_parent' as in the other error handling paths.
This is useless (priv->fixed_link is NULL at this point), but at least
it is consistent.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/ethernet/freescale/fman/mac.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fman/mac.c 
b/drivers/net/ethernet/freescale/fman/mac.c
index c27667a005f7..ca12e28129ed 100644
--- a/drivers/net/ethernet/freescale/fman/mac.c
+++ b/drivers/net/ethernet/freescale/fman/mac.c
@@ -709,11 +709,8 @@ static int mac_probe(struct platform_device *_of_dev)
}
 
if (!of_device_is_available(mac_node)) {
-   devm_iounmap(dev, priv->vaddr);
-   __devm_release_region(dev, fman_get_mem_region(priv->fman),
- res.start, res.end + 1 - res.start);
-   devm_kfree(dev, mac_dev);
-   return -ENODEV;
+   err = -ENODEV;
+   goto _return_of_get_parent;
}
 
/* Get the cell-index */
-- 
2.14.1

[PATCH 0/4] fsl/fman: Fix some error handling code in mac_probe

[Christophe JAILLET]

Commit c6e26ea8c893 ("dpaa_eth: change device used") generated some
conflicts in my patches waiting for submission. So I took a closer look at
it.


So here is a serie of 4 patches.

The 1st one is just about a spurious call to 'dev_set_drvdata()', which is
done in only 1 error handling path in the function.

The 2nd one removes some devm_iounmap/release/kfree functions which look
useless to me.

The 3rd one fixes a missing of_node_put.

The 4th one is just cosmetic and removes a useless message.


Christophe JAILLET (4):
  fsl/fman: Remove a useless call to 'dev_set_drvdata()'
  fsl/fman: Remove some useless code
  fsl/fman: Add a missing 'of_node_put()' call in an error handling path
  fsl/fman: Remove a useless 'dev_err()' call

 drivers/net/ethernet/freescale/fman/mac.c | 10 +++---
 1 file changed, 3 insertions(+), 7 deletions(-)

-- 
2.14.1

[PATCH] net: hns3: Fix an error handling path in 'hclge_rss_init_hw()'

[Christophe JAILLET]

If this sanity check fails, we must free 'rss_indir'. Otherwise there is a
memory leak.
'goto err' as done in the other error handling paths to fix it.

Fixes: 46a3df9f9718 ("net: hns3: Fix for setting rss_size incorrectly")
Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c 
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index e0685e630afe..c1cdbfd83bdb 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -2652,7 +2652,8 @@ static int hclge_rss_init_hw(struct hclge_dev *hdev)
dev_err(>pdev->dev,
"Configure rss tc size failed, invalid TC_SIZE = %d\n",
rss_size);
-   return -EINVAL;
+   ret = -EINVAL;
+   goto err;
}
 
roundup_size = roundup_pow_of_two(rss_size);
-- 
2.11.0

[PATCH] cnic: Fix an error handling path in 'cnic_alloc_bnx2x_resc()'

[Christophe JAILLET]

All the error handling paths 'goto error', except this one.
We should also go to error in this case, or some resources will be
leaking.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/ethernet/broadcom/cnic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/cnic.c 
b/drivers/net/ethernet/broadcom/cnic.c
index cec94bbb2ea5..8bc126a156e8 100644
--- a/drivers/net/ethernet/broadcom/cnic.c
+++ b/drivers/net/ethernet/broadcom/cnic.c
@@ -1278,7 +1278,7 @@ static int cnic_alloc_bnx2x_resc(struct cnic_dev *dev)
 
ret = cnic_alloc_dma(dev, kwq_16_dma, pages, 0);
if (ret)
-   return -ENOMEM;
+   goto error;
 
n = CNIC_PAGE_SIZE / CNIC_KWQ16_DATA_SIZE;
for (i = 0, j = 0; i < cp->max_cid_space; i++) {
-- 
2.11.0

[PATCH v2] openvswitch: Fix an error handling path in 'ovs_nla_init_match_and_action()'

[Christophe JAILLET]

All other error handling paths in this function go through the 'error'
label. This one should do the same.

Fixes: 9cc9a5cb176c ("datapath: Avoid using stack larger than 1024.")
Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
I think that the comment above the function could be improved. It looks
like the commit log which has introduced this function.

I'm also not sure that commit 9cc9a5cb176c is of any help. It is
supposed to remove a warning, and I guess it does. But 
'ovs_nla_init_match_and_action()'
is called unconditionnaly from 'ovs_flow_cmd_set()'. So even if the stack
used by each function is reduced, the overall stack should be the same, if
not larger.

So this commit sounds like adding a bug where the code was fine and states
to fix an issue but, at the best, only hides it.

Instead of fixing the code with the proposed patch, reverting the initial
commit could also be considered.

V2: update Subject line
---
 net/openvswitch/datapath.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 76cf273a56c7..c3aec6227c91 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -1112,7 +1112,8 @@ static int ovs_nla_init_match_and_action(struct net *net,
if (!a[OVS_FLOW_ATTR_KEY]) {
OVS_NLERR(log,
  "Flow key attribute not present in set 
flow.");
-   return -EINVAL;
+   error = -EINVAL;
+   goto error;
}
 
*acts = get_flow_actions(net, a[OVS_FLOW_ATTR_ACTIONS], key,
-- 
2.11.0

[PATCH] datapath: Fix an error handling path in 'ovs_nla_init_match_and_action()'

[Christophe JAILLET]

All other error handling paths in this function go through the 'error'
label. This one should do the same.

Fixes: 9cc9a5cb176c ("datapath: Avoid using stack larger than 1024.")
Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
I think that the comment above the function could be improved. It looks
like the commit log which has introduced this function.

I'm also not sure that commit 9cc9a5cb176c is of any help. It is
supposed to remove a warning, and I guess it does. But 
'ovs_nla_init_match_and_action()'
is called unconditionnaly from 'ovs_flow_cmd_set()'. So even if the stack
used by each function is reduced, the overall stack should be the same, if
not larger.

So this commit sounds like adding a bug where the code was fine and states
to fix an issue but, at the best, only hides it.

Instead of fixing the code with the proposed patch, reverting the initial
commit could also be considered.
---
 net/openvswitch/datapath.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 76cf273a56c7..c3aec6227c91 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -1112,7 +1112,8 @@ static int ovs_nla_init_match_and_action(struct net *net,
if (!a[OVS_FLOW_ATTR_KEY]) {
OVS_NLERR(log,
  "Flow key attribute not present in set 
flow.");
-   return -EINVAL;
+   error = -EINVAL;
+   goto error;
}
 
*acts = get_flow_actions(net, a[OVS_FLOW_ATTR_ACTIONS], key,
-- 
2.11.0

Re: [PATCH] igb: check memory allocation failure

[Christophe JAILLET]

Le 28/08/2017 à 01:09, Waskiewicz Jr, Peter a écrit :

On 8/27/17 2:42 AM, Christophe JAILLET wrote:

Check memory allocation failures and return -ENOMEM in such cases, as
already done for other memory allocations in this function.

This avoids NULL pointers dereference.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
   drivers/net/ethernet/intel/igb/igb_main.c | 2 ++
   1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c 
b/drivers/net/ethernet/intel/igb/igb_main.c
index fd4a46b03cc8..837d9b46a390 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -3162,6 +3162,8 @@ static int igb_sw_init(struct igb_adapter *adapter)
/* Setup and initialize a copy of the hw vlan table array */
adapter->shadow_vfta = kcalloc(E1000_VLAN_FILTER_TBL_SIZE, sizeof(u32),
   GFP_ATOMIC);
+   if (!adapter->shadow_vfta)
+   return -ENOMEM;

Looks reasonable to me.

A larger issue though I see in this function is that if we return
-ENOMEM here, and if we return -ENOMEM from igb_init_interrupt_scheme()
below on failure, we leak adapter->mac_table (and adapter->shadow_vfta
in the latter).  We should add a proper unwind to free up the memory on
failure.

-PJ


Hi,

in fact, there is no leak because the only caller of 'igb_sw_init()' 
(i.e. 'igb_probe()'), already frees these resources in case of error, 
see [1]


These resources are also freed  in 'igb_remove()'.

Best reagrds,
CJ

[1]: 
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/net/ethernet/intel/igb/igb_main.c#n2775

[PATCH] igb: check memory allocation failure

[Christophe JAILLET]

Check memory allocation failures and return -ENOMEM in such cases, as
already done for other memory allocations in this function.

This avoids NULL pointers dereference.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/ethernet/intel/igb/igb_main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c 
b/drivers/net/ethernet/intel/igb/igb_main.c
index fd4a46b03cc8..837d9b46a390 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -3162,6 +3162,8 @@ static int igb_sw_init(struct igb_adapter *adapter)
/* Setup and initialize a copy of the hw vlan table array */
adapter->shadow_vfta = kcalloc(E1000_VLAN_FILTER_TBL_SIZE, sizeof(u32),
   GFP_ATOMIC);
+   if (!adapter->shadow_vfta)
+   return -ENOMEM;
 
/* This call may decrease the number of queues */
if (igb_init_interrupt_scheme(adapter, true)) {
-- 
2.11.0

[PATCH] net: sxgbe: check memory allocation failure

[Christophe JAILLET]

Check memory allocation failure and return -ENOMEM in such a case, as
already done few lines below for another memory allocation.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/ethernet/samsung/sxgbe/sxgbe_platform.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/samsung/sxgbe/sxgbe_platform.c 
b/drivers/net/ethernet/samsung/sxgbe/sxgbe_platform.c
index 73427e29df2a..fbd00cb0cb7d 100644
--- a/drivers/net/ethernet/samsung/sxgbe/sxgbe_platform.c
+++ b/drivers/net/ethernet/samsung/sxgbe/sxgbe_platform.c
@@ -47,6 +47,8 @@ static int sxgbe_probe_config_dt(struct platform_device *pdev,
plat->mdio_bus_data = devm_kzalloc(>dev,
   sizeof(*plat->mdio_bus_data),
   GFP_KERNEL);
+   if (!plat->mdio_bus_data)
+   return -ENOMEM;
 
dma_cfg = devm_kzalloc(>dev, sizeof(*dma_cfg), GFP_KERNEL);
if (!dma_cfg)
-- 
2.11.0

Re: [PATCH] mt7601u: check memory allocation failure

[Christophe JAILLET]

Le 21/08/2017 à 23:41, Jakub Kicinski a écrit :

On Mon, 21 Aug 2017 14:34:30 -0700, Jakub Kicinski wrote:

On Mon, 21 Aug 2017 22:59:56 +0200, Christophe JAILLET wrote:

Check memory allocation failure and return -ENOMEM in such a case, as
already done a few lines below

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>

Acked-by: Jakub Kicinski <kubak...@wp.pl>

Wait, I take that back.  This code is a bit weird.  We would return an
error, then mt7601u_dma_init() will call mt7601u_free_tx_queue() which
doesn't check for tx_q == NULL condition.

Looks like mt7601u_free_tx() has to check for dev->tx_q == NULL and
return early if that's the case.  Or mt7601u_alloc_tx() should really
clean things up on it's own on failure.  Ugh.


You are right. Thanks for the review.

I've sent a v2 which updates 'mt7601u_free_tx()'.
Doing so sounds more in line with the spirit of this code.

CJ

[PATCH v2] mt7601u: check memory allocation failure

[Christophe JAILLET]

Check memory allocation failure and return -ENOMEM in such a case, as
already done a few lines below.

As 'dev->tx_q' can be NULL, we also need to check for that in
'mt7601u_free_tx()', and return early.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
v2: avoid another NULL pointer dereference in 'mt7601u_free_tx()' if the
allocation had failed.
---
 drivers/net/wireless/mediatek/mt7601u/dma.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.c 
b/drivers/net/wireless/mediatek/mt7601u/dma.c
index 660267b359e4..7f3e3983b781 100644
--- a/drivers/net/wireless/mediatek/mt7601u/dma.c
+++ b/drivers/net/wireless/mediatek/mt7601u/dma.c
@@ -457,6 +457,9 @@ static void mt7601u_free_tx(struct mt7601u_dev *dev)
 {
int i;
 
+   if (!dev->tx_q)
+   return;
+
for (i = 0; i < __MT_EP_OUT_MAX; i++)
mt7601u_free_tx_queue(>tx_q[i]);
 }
@@ -484,6 +487,8 @@ static int mt7601u_alloc_tx(struct mt7601u_dev *dev)
 
dev->tx_q = devm_kcalloc(dev->dev, __MT_EP_OUT_MAX,
 sizeof(*dev->tx_q), GFP_KERNEL);
+   if (!dev->tx_q)
+   return -ENOMEM;
 
for (i = 0; i < __MT_EP_OUT_MAX; i++)
if (mt7601u_alloc_tx_queue(dev, >tx_q[i]))
-- 
2.11.0

[PATCH] mt7601u: check memory allocation failure

[Christophe JAILLET]

Check memory allocation failure and return -ENOMEM in such a case, as
already done a few lines below

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/wireless/mediatek/mt7601u/dma.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.c 
b/drivers/net/wireless/mediatek/mt7601u/dma.c
index 660267b359e4..fa0173579d32 100644
--- a/drivers/net/wireless/mediatek/mt7601u/dma.c
+++ b/drivers/net/wireless/mediatek/mt7601u/dma.c
@@ -484,6 +484,8 @@ static int mt7601u_alloc_tx(struct mt7601u_dev *dev)
 
dev->tx_q = devm_kcalloc(dev->dev, __MT_EP_OUT_MAX,
 sizeof(*dev->tx_q), GFP_KERNEL);
+   if (!dev->tx_q)
+   return -ENOMEM;
 
for (i = 0; i < __MT_EP_OUT_MAX; i++)
if (mt7601u_alloc_tx_queue(dev, >tx_q[i]))
-- 
2.11.0

[PATCH] ieee802154: ca8210: Fix a potential NULL pointer dereference

[Christophe JAILLET]

'spi' is known to be NULL, so we dereference a NULL pointer here.
Use 'pr_crit()' instead of 'dev_crit()' to report the message.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/ieee802154/ca8210.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
index 326243fae7e2..24a1eabbbc9d 100644
--- a/drivers/net/ieee802154/ca8210.c
+++ b/drivers/net/ieee802154/ca8210.c
@@ -917,10 +917,7 @@ static int ca8210_spi_transfer(
struct cas_control *cas_ctl;
 
if (!spi) {
-   dev_crit(
-   >dev,
-   "NULL spi device passed to ca8210_spi_transfer\n"
-   );
+   pr_crit("NULL spi device passed to %s\n", __func__);
return -ENODEV;
}
 
-- 
2.11.0

Re: [PATCH] net: ibm: emac: Fix some error handling path in 'emac_probe()'

[Christophe JAILLET]

Le 19/08/2017 à 15:22, Christian Lamparter a écrit :

On Saturday, August 19, 2017 1:07:57 AM CEST Christophe JAILLET wrote:

If 'irq_of_parse_and_map()' or 'of_address_to_resource()' fail, 'err' is
known to be 0 at this point.
So return -ENODEV instead in the first case and propagate the error
returned by 'of_address_to_resource()' in the 2nd case.

While at it, turn a 'err != 0' test into an equivalent 'err' to be more
consistent.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
  drivers/net/ethernet/ibm/emac/core.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/ibm/emac/core.c 
b/drivers/net/ethernet/ibm/emac/core.c
index 95135d20458f..1af56a97fb47 100644
--- a/drivers/net/ethernet/ibm/emac/core.c
+++ b/drivers/net/ethernet/ibm/emac/core.c
[...]
/* Map EMAC regs */
-   if (of_address_to_resource(np, 0, >rsrc_regs)) {
+   err = of_address_to_resource(np, 0, >rsrc_regs);
+   if (err) {
printk(KERN_ERR "%pOF: Can't get registers address\n", np);
goto err_irq_unmap;
}
  // TODO : request_mem_region
  dev->emacp = ioremap(dev->rsrc_regs.start,
   resource_size(>rsrc_regs));
  ...

If you want to go for 101%: you could get rid of this block
altogether by doing:
dev->emacp = of_iomap(np, 0);

Note1:
This will also make the rsrc_regs variable in the emac_instance
struct redundant. So simply remove it from the core.h.

Note2: if you want to go for 110%, you could replace this with
platform_get_resource() and devm_ioremap_resource() (if you
are interested, take a look at devm_ioremap_resource's kdoc
it has an example).

Thanks,
Christian


Hi,

Thanks for the review and comments.

I've sent a v2 to go for 101% which axes some lines of code.

I won't propose anything for your other proposal. Sounds great but 
involves more changes in the error handling path and in the remove function.

I don't have the hardware, so I won't be able to test this bigger change.

Christophe

[PATCH v2] net: ibm: emac: Fix some error handling path in 'emac_probe()'

[Christophe JAILLET]

If 'irq_of_parse_and_map()' or 'of_address_to_resource()' fail, 'err' is
known to be 0 at this point.
So return -ENODEV instead in the first case and use 'of_iomap()' instead of
the equivalent 'of_address_to_resource()/ioremap()' combinaison in the 2nd
case.

Doing so, the 'rsrc_regs' field of the 'emac_instance struct' becomes
redundant and is removed.

While at it, turn a 'err != 0' test into an equivalent 'err' to be more
consistent.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
v2: use of_iomap() to simplify code
remove 'rsrc_regs' field of the 'emac_instance struct'
update comment
---
 drivers/net/ethernet/ibm/emac/core.c | 12 
 drivers/net/ethernet/ibm/emac/core.h |  1 -
 2 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/ibm/emac/core.c 
b/drivers/net/ethernet/ibm/emac/core.c
index 95135d20458f..7feff2450ed6 100644
--- a/drivers/net/ethernet/ibm/emac/core.c
+++ b/drivers/net/ethernet/ibm/emac/core.c
@@ -3032,7 +3032,7 @@ static int emac_probe(struct platform_device *ofdev)
 
/* Init various config data based on device-tree */
err = emac_init_config(dev);
-   if (err != 0)
+   if (err)
goto err_free;
 
/* Get interrupts. EMAC irq is mandatory, WOL irq is optional */
@@ -3040,18 +3040,14 @@ static int emac_probe(struct platform_device *ofdev)
dev->wol_irq = irq_of_parse_and_map(np, 1);
if (!dev->emac_irq) {
printk(KERN_ERR "%pOF: Can't map main interrupt\n", np);
+   err = -ENODEV;
goto err_free;
}
ndev->irq = dev->emac_irq;
 
/* Map EMAC regs */
-   if (of_address_to_resource(np, 0, >rsrc_regs)) {
-   printk(KERN_ERR "%pOF: Can't get registers address\n", np);
-   goto err_irq_unmap;
-   }
-   // TODO : request_mem_region
-   dev->emacp = ioremap(dev->rsrc_regs.start,
-resource_size(>rsrc_regs));
+   // TODO : platform_get_resource() and devm_ioremap_resource()
+   dev->emacp = of_iomap(np, 0);
if (dev->emacp == NULL) {
printk(KERN_ERR "%pOF: Can't map device registers!\n", np);
err = -ENOMEM;
diff --git a/drivers/net/ethernet/ibm/emac/core.h 
b/drivers/net/ethernet/ibm/emac/core.h
index f10e156641d5..369de2cfb15b 100644
--- a/drivers/net/ethernet/ibm/emac/core.h
+++ b/drivers/net/ethernet/ibm/emac/core.h
@@ -167,7 +167,6 @@ struct emac_error_stats {
 
 struct emac_instance {
struct net_device   *ndev;
-   struct resource rsrc_regs;
struct emac_regs__iomem *emacp;
struct platform_device  *ofdev;
struct device_node  **blist; /* bootlist entry */
-- 
2.11.0

[PATCH] net: ibm: emac: Fix some error handling path in 'emac_probe()'

[Christophe JAILLET]

If 'irq_of_parse_and_map()' or 'of_address_to_resource()' fail, 'err' is
known to be 0 at this point.
So return -ENODEV instead in the first case and propagate the error
returned by 'of_address_to_resource()' in the 2nd case.

While at it, turn a 'err != 0' test into an equivalent 'err' to be more
consistent.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/ethernet/ibm/emac/core.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/ibm/emac/core.c 
b/drivers/net/ethernet/ibm/emac/core.c
index 95135d20458f..1af56a97fb47 100644
--- a/drivers/net/ethernet/ibm/emac/core.c
+++ b/drivers/net/ethernet/ibm/emac/core.c
@@ -3032,7 +3032,7 @@ static int emac_probe(struct platform_device *ofdev)
 
/* Init various config data based on device-tree */
err = emac_init_config(dev);
-   if (err != 0)
+   if (err)
goto err_free;
 
/* Get interrupts. EMAC irq is mandatory, WOL irq is optional */
@@ -3040,12 +3040,14 @@ static int emac_probe(struct platform_device *ofdev)
dev->wol_irq = irq_of_parse_and_map(np, 1);
if (!dev->emac_irq) {
printk(KERN_ERR "%pOF: Can't map main interrupt\n", np);
+   err = -ENODEV;
goto err_free;
}
ndev->irq = dev->emac_irq;
 
/* Map EMAC regs */
-   if (of_address_to_resource(np, 0, >rsrc_regs)) {
+   err = of_address_to_resource(np, 0, >rsrc_regs);
+   if (err) {
printk(KERN_ERR "%pOF: Can't get registers address\n", np);
goto err_irq_unmap;
}
-- 
2.11.0

[PATCH] qed: Fix a memory allocation failure test in 'qed_mcp_cmd_init()'

[Christophe JAILLET]

We allocate 'p_info->mfw_mb_cur' and 'p_info->mfw_mb_shadow' but we check
'p_info->mfw_mb_addr' instead of 'p_info->mfw_mb_cur'.

'p_info->mfw_mb_addr' is never 0, because it is initiliazed a few lines
above in 'qed_load_mcp_offsets()'.

Update the test and check the result of the 2 'kzalloc()' instead.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/ethernet/qlogic/qed/qed_mcp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_mcp.c 
b/drivers/net/ethernet/qlogic/qed/qed_mcp.c
index c1ecce6b9141..376485d99357 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_mcp.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_mcp.c
@@ -253,7 +253,7 @@ int qed_mcp_cmd_init(struct qed_hwfn *p_hwfn, struct 
qed_ptt *p_ptt)
size = MFW_DRV_MSG_MAX_DWORDS(p_info->mfw_mb_length) * sizeof(u32);
p_info->mfw_mb_cur = kzalloc(size, GFP_KERNEL);
p_info->mfw_mb_shadow = kzalloc(size, GFP_KERNEL);
-   if (!p_info->mfw_mb_shadow || !p_info->mfw_mb_addr)
+   if (!p_info->mfw_mb_cur || !p_info->mfw_mb_shadow)
goto err;
 
return 0;
-- 
2.11.0

[PATCH] i40e: Fix a potential NULL pointer dereference

[Christophe JAILLET]

If 'kzalloc()' fails, a NULL pointer will be dereferenced.
Return an error code (-ENOMEM) instead.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c 
b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index 979110d59f67..facd48fc15d4 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -423,6 +423,9 @@ static int i40e_config_iwarp_qvlist(struct i40e_vf *vf,
   (sizeof(struct virtchnl_iwarp_qv_info) *
(qvlist_info->num_vectors - 1));
vf->qvlist_info = kzalloc(size, GFP_KERNEL);
+   if (!vf->qvlist_info)
+   return -ENOMEM;
+
vf->qvlist_info->num_vectors = qvlist_info->num_vectors;
 
msix_vf = pf->hw.func_caps.num_msix_vectors_vf;
-- 
2.11.0

[PATCH] atm: zatm: Fix an error handling path in 'zatm_init_one()'

[Christophe JAILLET]

If 'dma_set_mask_and_coherent()' fails, we must undo the previous
'pci_request_regions()' call.
Adjust corresponding 'goto' to jump at the right place of the error
handling path.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/atm/zatm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/atm/zatm.c b/drivers/atm/zatm.c
index 292dec18ffb8..07bdd51b3b9a 100644
--- a/drivers/atm/zatm.c
+++ b/drivers/atm/zatm.c
@@ -1613,7 +1613,7 @@ static int zatm_init_one(struct pci_dev *pci_dev,
 
ret = dma_set_mask_and_coherent(_dev->dev, DMA_BIT_MASK(32));
if (ret < 0)
-   goto out_disable;
+   goto out_release;
 
zatm_dev->pci_dev = pci_dev;
dev->dev_data = zatm_dev;
-- 
2.11.0

[PATCH v2] iwlwifi: mvm: Fix a memory leak in an error handling path in 'iwl_mvm_sar_get_wgds_table()'

[Christophe JAILLET]

We should free 'wgds.pointer' here as done a few lines above in another
error handling path.
It was allocated within 'acpi_evaluate_object()'.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
v2: rebase after 7fe90e0e3d60 ("iwlwifi: mvm: refactor geo init")


Moreovern a comment in '/drivers/acpi/acpica/utalloc.c' states that:
/* [...] Note: The caller should use acpi_os_free to free this
 * buffer created via ACPI_ALLOCATE_BUFFER.
 */

So, at the end of this function:
out_free:
kfree(wgds.pointer);
should maybe be:
out_free:
acpi_os_free(wgds.pointer);

If correct, several places should be fixed accordingly.
---
 drivers/net/wireless/intel/iwlwifi/mvm/fw.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c 
b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
index 79e7a7a285dc..82863e9273eb 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
@@ -1275,8 +1275,10 @@ static int iwl_mvm_sar_get_wgds_table(struct iwl_mvm 
*mvm)
 
entry = _pkg->package.elements[idx++];
if ((entry->type != ACPI_TYPE_INTEGER) ||
-   (entry->integer.value > U8_MAX))
-   return -EINVAL;
+   (entry->integer.value > U8_MAX)) {
+   ret = -EINVAL;
+   goto out_free;
+   }
 
mvm->geo_profiles[i].values[j] = entry->integer.value;
}
-- 
2.11.0

[PATCH] iwlwifi: mvm: Fix a memory leak in an error handling path in 'iwl_mvm_sar_get_wgds_table()'

[Christophe JAILLET]

We should free 'wgds.pointer' here as done a few lines above in another
error handling path.
It was allocated within 'acpi_evaluate_object()'.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
A comment in '/drivers/acpi/acpica/utalloc.c' states that:
/* [...] Note: The caller should use acpi_os_free to free this
 * buffer created via ACPI_ALLOCATE_BUFFER.
 */

So, at the end of this function:
out_free:
kfree(wgds.pointer);
should maybe be:
out_free:
acpi_os_free(wgds.pointer);

If correct, several places should be fixed accordingly.
---
 drivers/net/wireless/intel/iwlwifi/mvm/fw.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c 
b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
index 24cc406d87ef..56d535ab696f 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
@@ -1287,8 +1287,10 @@ static int iwl_mvm_sar_get_wgds_table(struct iwl_mvm 
*mvm,
 
entry = _pkg->package.elements[i + 1];
if ((entry->type != ACPI_TYPE_INTEGER) ||
-   (entry->integer.value > U8_MAX))
-   return -EINVAL;
+   (entry->integer.value > U8_MAX)) {
+   ret = -EINVAL;
+   goto out_free;
+   }
 
geo_table->values[i] = entry->integer.value;
}
-- 
2.11.0

[PATCH v2] mrf24j40: Fix en error handling path in 'mrf24j40_probe()'

[Christophe JAILLET]

If this check fails, we must release some resources as done everywhere
else in this function before returning an error code.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
V2: initialization of ret in this erro path ws missing. Stupid me!
---
 drivers/net/ieee802154/mrf24j40.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ieee802154/mrf24j40.c 
b/drivers/net/ieee802154/mrf24j40.c
index 7d334963dc08..da8683782ffc 100644
--- a/drivers/net/ieee802154/mrf24j40.c
+++ b/drivers/net/ieee802154/mrf24j40.c
@@ -1330,7 +1330,8 @@ static int mrf24j40_probe(struct spi_device *spi)
if (spi->max_speed_hz > MAX_SPI_SPEED_HZ) {
dev_warn(>dev, "spi clock above possible maximum: %d",
 MAX_SPI_SPEED_HZ);
-   return -EINVAL;
+   ret = -EINVAL;
+   goto err_register_device;
}
 
ret = mrf24j40_hw_init(devrec);
-- 
2.11.0

[PATCH] mrf24j40: Fix en error handling path in 'mrf24j40_probe()'

[Christophe JAILLET]

If this check fails, we must release some resources as done everywhere
else in this function before returning an error code.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/ieee802154/mrf24j40.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ieee802154/mrf24j40.c 
b/drivers/net/ieee802154/mrf24j40.c
index 7d334963dc08..da8683782ffc 100644
--- a/drivers/net/ieee802154/mrf24j40.c
+++ b/drivers/net/ieee802154/mrf24j40.c
@@ -1330,7 +1330,7 @@ static int mrf24j40_probe(struct spi_device *spi)
if (spi->max_speed_hz > MAX_SPI_SPEED_HZ) {
dev_warn(>dev, "spi clock above possible maximum: %d",
 MAX_SPI_SPEED_HZ);
-   return -EINVAL;
+   goto err_register_device;
}
 
ret = mrf24j40_hw_init(devrec);
-- 
2.11.0

[PATCH 3/3] net: stmmac: Make 'alloc_dma_[rt]x_desc_resources()' look even closer

[Christophe JAILLET]

'alloc_dma_[rt]x_desc_resources()' functions look very close.
Remove a useless initialization and use the same label name for error
handling path in order to get them even closer.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 07d486a70118..1853f7ff6657 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1449,7 +1449,7 @@ static void free_dma_rx_desc_resources(struct stmmac_priv 
*priv)
 static void free_dma_tx_desc_resources(struct stmmac_priv *priv)
 {
u32 tx_count = priv->plat->tx_queues_to_use;
-   u32 queue = 0;
+   u32 queue;
 
/* Free TX queue resources */
for (queue = 0; queue < tx_count; queue++) {
@@ -1561,13 +1561,13 @@ static int alloc_dma_tx_desc_resources(struct 
stmmac_priv *priv)

sizeof(*tx_q->tx_skbuff_dma),
GFP_KERNEL);
if (!tx_q->tx_skbuff_dma)
-   goto err_dma_buffers;
+   goto err_dma;
 
tx_q->tx_skbuff = kmalloc_array(DMA_TX_SIZE,
sizeof(struct sk_buff *),
GFP_KERNEL);
if (!tx_q->tx_skbuff)
-   goto err_dma_buffers;
+   goto err_dma;
 
if (priv->extend_desc) {
tx_q->dma_etx = dma_zalloc_coherent(priv->device,
@@ -1577,7 +1577,7 @@ static int alloc_dma_tx_desc_resources(struct stmmac_priv 
*priv)
_q->dma_tx_phy,
GFP_KERNEL);
if (!tx_q->dma_etx)
-   goto err_dma_buffers;
+   goto err_dma;
} else {
tx_q->dma_tx = dma_zalloc_coherent(priv->device,
   DMA_TX_SIZE *
@@ -1586,13 +1586,13 @@ static int alloc_dma_tx_desc_resources(struct 
stmmac_priv *priv)
   _q->dma_tx_phy,
   GFP_KERNEL);
if (!tx_q->dma_tx)
-   goto err_dma_buffers;
+   goto err_dma;
}
}
 
return 0;
 
-err_dma_buffers:
+err_dma:
free_dma_tx_desc_resources(priv);
 
return ret;
-- 
2.11.0

[PATCH 1/3] net: stmmac: Fix error handling path in 'alloc_dma_rx_desc_resources()'

[Christophe JAILLET]

If the first 'kmalloc_array' within the loop fails, we should free what
as already been allocated, as done in all other error handling path.

Fixes: 54139cf3bb33 ("net: stmmac: adding multiple buffers for rx")
Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 19bba6281dab..4322fa4a13e8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1498,7 +1498,7 @@ static int alloc_dma_rx_desc_resources(struct stmmac_priv 
*priv)
sizeof(dma_addr_t),
GFP_KERNEL);
if (!rx_q->rx_skbuff_dma)
-   return -ENOMEM;
+   goto err_dma;
 
rx_q->rx_skbuff = kmalloc_array(DMA_RX_SIZE,
sizeof(struct sk_buff *),
-- 
2.11.0

[PATCH 2/3] net: stmmac: Fix error handling path in 'alloc_dma_tx_desc_resources()'

[Christophe JAILLET]

If the first 'kmalloc_array' within the loop fails, we should free what
as already been allocated, as done in all other error handling path.

Fixes: ce736788e8a9 ("net: stmmac: adding multiple buffers for TX")
Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 4322fa4a13e8..07d486a70118 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1561,7 +1561,7 @@ static int alloc_dma_tx_desc_resources(struct stmmac_priv 
*priv)

sizeof(*tx_q->tx_skbuff_dma),
GFP_KERNEL);
if (!tx_q->tx_skbuff_dma)
-   return -ENOMEM;
+   goto err_dma_buffers;
 
tx_q->tx_skbuff = kmalloc_array(DMA_TX_SIZE,
sizeof(struct sk_buff *),
-- 
2.11.0

[PATCH 0/3] net: stmmac: Fixes and cleanups in 'alloc_dma_[rt]x_desc_resources()'

[Christophe JAILLET]

These patchs are all related to 'alloc_dma_[rt]x_desc_resources()' functions.

The 2 first fix an error path where some resources are leaking. I've
separated them into 2 patches because the issues have been introduced by
2 deferent commits.

The 3rd patch is just a clean-up.

Christophe JAILLET (3):
  net: stmmac: Fix error handling path in
'alloc_dma_rx_desc_resources()'
  net: stmmac: Fix error handling path in
'alloc_dma_tx_desc_resources()'
  net: stmmac: Make 'alloc_dma_[rt]x_desc_resources()' look even closer

 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

-- 
2.11.0

[PATCH] cisco: enic: Fic an error handling path in 'vnic_dev_init_devcmd2()'

[Christophe JAILLET]

if 'ioread32()' returns 0xFFF, we have to go through the error
handling path as done everywhere else in this function.

Move the 'err_free_wq' label to better match its name and its location
and add a new label 'err_disable_wq'.
Update the code accordingly.

Fixes: 373fb0873d43 ("enic: add devcmd2")
Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/ethernet/cisco/enic/vnic_dev.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/cisco/enic/vnic_dev.c 
b/drivers/net/ethernet/cisco/enic/vnic_dev.c
index 1841ad45d215..39bad67422dd 100644
--- a/drivers/net/ethernet/cisco/enic/vnic_dev.c
+++ b/drivers/net/ethernet/cisco/enic/vnic_dev.c
@@ -402,8 +402,8 @@ static int vnic_dev_init_devcmd2(struct vnic_dev *vdev)
fetch_index = ioread32(>devcmd2->wq.ctrl->fetch_index);
if (fetch_index == 0x) { /* check for hardware gone  */
vdev_err(vdev, "Fatal error in devcmd2 init - hardware surprise 
removal\n");
-
-   return -ENODEV;
+   err = -ENODEV;
+   goto err_free_wq;
}
 
enic_wq_init_start(>devcmd2->wq, 0, fetch_index, fetch_index, 0,
@@ -414,7 +414,7 @@ static int vnic_dev_init_devcmd2(struct vnic_dev *vdev)
err = vnic_dev_alloc_desc_ring(vdev, >devcmd2->results_ring,
   DEVCMD2_RING_SIZE, DEVCMD2_DESC_SIZE);
if (err)
-   goto err_free_wq;
+   goto err_disable_wq;
 
vdev->devcmd2->result = vdev->devcmd2->results_ring.descs;
vdev->devcmd2->cmd_ring = vdev->devcmd2->wq.ring.descs;
@@ -433,8 +433,9 @@ static int vnic_dev_init_devcmd2(struct vnic_dev *vdev)
 
 err_free_desc_ring:
vnic_dev_free_desc_ring(vdev, >devcmd2->results_ring);
-err_free_wq:
+err_disable_wq:
vnic_wq_disable(>devcmd2->wq);
+err_free_wq:
vnic_wq_free(>devcmd2->wq);
 err_free_devcmd2:
kfree(vdev->devcmd2);
-- 
2.11.0

[PATCH] arcnet: com20020-pci: Fix an error handling path in 'com20020pci_probe()'

[Christophe JAILLET]

If this memory allocation fails, we should go through the error handling
path as done everywhere else in this function before returning.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/arcnet/com20020-pci.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/arcnet/com20020-pci.c 
b/drivers/net/arcnet/com20020-pci.c
index 239de38fbd6a..8e89a2ac071e 100644
--- a/drivers/net/arcnet/com20020-pci.c
+++ b/drivers/net/arcnet/com20020-pci.c
@@ -196,8 +196,10 @@ static int com20020pci_probe(struct pci_dev *pdev,
 
card = devm_kzalloc(>dev, sizeof(struct com20020_dev),
GFP_KERNEL);
-   if (!card)
-   return -ENOMEM;
+   if (!card) {
+   ret = -ENOMEM;
+   goto out_port;
+   }
 
card->index = i;
card->pci_priv = priv;
-- 
2.11.0

[PATCH v2] brcmfmac: Fix a memory leak in error handling path in 'brcmf_cfg80211_attach'

[Christophe JAILLET]

If 'wiphy_new()' fails, we leak 'ops'. Add a new label in the error
handling path to free it in such a case.

Cc: sta...@vger.kernel.org
Fixes: 5c22fb85102a7 ("brcmfmac: add wowl gtk rekeying offload support")
Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
v2: Add CC tag
Change prefix
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 2443c71a202f..032d823c53c2 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -6861,7 +6861,7 @@ struct brcmf_cfg80211_info *brcmf_cfg80211_attach(struct 
brcmf_pub *drvr,
wiphy = wiphy_new(ops, sizeof(struct brcmf_cfg80211_info));
if (!wiphy) {
brcmf_err("Could not allocate wiphy device\n");
-   return NULL;
+   goto ops_out;
}
memcpy(wiphy->perm_addr, drvr->mac, ETH_ALEN);
set_wiphy_dev(wiphy, busdev);
@@ -7012,6 +7012,7 @@ struct brcmf_cfg80211_info *brcmf_cfg80211_attach(struct 
brcmf_pub *drvr,
ifp->vif = NULL;
 wiphy_out:
brcmf_free_wiphy(wiphy);
+ops_out:
kfree(ops);
return NULL;
 }
-- 
2.11.0

[PATCH] cfg80211: Fix a memory leak in error handling path in 'brcmf_cfg80211_attach'

[Christophe JAILLET]

If 'wiphy_new()' fails, we leak 'ops'. Add a new label in the error
handling path to free it in such a case.

Fixes: 5c22fb85102a7 ("brcmfmac: add wowl gtk rekeying offload support")
Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 2443c71a202f..032d823c53c2 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -6861,7 +6861,7 @@ struct brcmf_cfg80211_info *brcmf_cfg80211_attach(struct 
brcmf_pub *drvr,
wiphy = wiphy_new(ops, sizeof(struct brcmf_cfg80211_info));
if (!wiphy) {
brcmf_err("Could not allocate wiphy device\n");
-   return NULL;
+   goto ops_out;
}
memcpy(wiphy->perm_addr, drvr->mac, ETH_ALEN);
set_wiphy_dev(wiphy, busdev);
@@ -7012,6 +7012,7 @@ struct brcmf_cfg80211_info *brcmf_cfg80211_attach(struct 
brcmf_pub *drvr,
ifp->vif = NULL;
 wiphy_out:
brcmf_free_wiphy(wiphy);
+ops_out:
kfree(ops);
return NULL;
 }
-- 
2.11.0

[PATCH] net: dsa: loop: Free resources if initialization is deferred

[Christophe JAILLET]

Free some devm'allocated memory in case of deferred driver initialization.
This avoid to waste some memory in such a case.

Suggested-by: Joe Perches <j...@perches.com>
Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/dsa/dsa_loop.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/dsa_loop.c b/drivers/net/dsa/dsa_loop.c
index a19e1781e9bb..557afb418320 100644
--- a/drivers/net/dsa/dsa_loop.c
+++ b/drivers/net/dsa/dsa_loop.c
@@ -260,8 +260,11 @@ static int dsa_loop_drv_probe(struct mdio_device *mdiodev)
return -ENOMEM;
 
ps->netdev = dev_get_by_name(_net, pdata->netdev);
-   if (!ps->netdev)
+   if (!ps->netdev) {
+   devm_kfree(>dev, ps);
+   devm_kfree(>dev, ds);
return -EPROBE_DEFER;
+   }
 
pdata->cd.netdev[DSA_LOOP_CPU_PORT] = >netdev->dev;
 
-- 
2.11.0

[PATCH] net: dsa: loop: Check for memory allocation failure

[Christophe JAILLET]

If 'devm_kzalloc' fails, a NULL pointer will be dereferenced.
Return -ENOMEM instead, as done for some other memory allocation just a
few lines above.

Fixes: 98cd1552ea27 ("net: dsa: Mock-up driver")

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/dsa/dsa_loop.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/dsa/dsa_loop.c b/drivers/net/dsa/dsa_loop.c
index f0fc4de4fc9a..a19e1781e9bb 100644
--- a/drivers/net/dsa/dsa_loop.c
+++ b/drivers/net/dsa/dsa_loop.c
@@ -256,6 +256,9 @@ static int dsa_loop_drv_probe(struct mdio_device *mdiodev)
return -ENOMEM;
 
ps = devm_kzalloc(>dev, sizeof(*ps), GFP_KERNEL);
+   if (!ps)
+   return -ENOMEM;
+
ps->netdev = dev_get_by_name(_net, pdata->netdev);
if (!ps->netdev)
return -EPROBE_DEFER;
-- 
2.11.0

[PATCH] wcn36xx: Fix error handling

[Christophe JAILLET]

Reorder 'out_free_dxe_pool' and 'out_free_dxe_ctl' error handling labels
in order to match the way resources have been allocated.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/wireless/ath/wcn36xx/main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/wcn36xx/main.c 
b/drivers/net/wireless/ath/wcn36xx/main.c
index cee4f655bf36..6436235bc265 100644
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -337,10 +337,10 @@ static int wcn36xx_start(struct ieee80211_hw *hw)
wcn36xx_smd_stop(wcn);
 out_free_smd_buf:
kfree(wcn->hal_buf);
-out_free_dxe_pool:
-   wcn36xx_dxe_free_mem_pools(wcn);
 out_free_dxe_ctl:
wcn36xx_dxe_free_ctl_blks(wcn);
+out_free_dxe_pool:
+   wcn36xx_dxe_free_mem_pools(wcn);
 out_smd_close:
wcn36xx_smd_close(wcn);
 out_err:
-- 
2.9.3

[PATCH] qlcnic: Fix a memory leak in error handling path

[Christophe JAILLET]

If 'dma_alloc_coherent()' fails, we should release resources allocated so
far, just as done in all other cases in this function.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
checkpatch.pl complains about '== NULL'. I have left it as-is because both
'== NULL' and '!' are already used in the file.
---
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c 
b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c
index daf05155b732..d344e9d43832 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c
@@ -573,8 +573,10 @@ int qlcnic_alloc_hw_resources(struct qlcnic_adapter 
*adapter)
ptr = (__le32 *)dma_alloc_coherent(>dev, sizeof(u32),
   _ring->hw_cons_phys_addr,
   GFP_KERNEL);
-   if (ptr == NULL)
-   return -ENOMEM;
+   if (ptr == NULL) {
+   err = -ENOMEM;
+   goto err_out_free;
+   }
 
tx_ring->hw_consumer = ptr;
/* cmd desc ring */
-- 
2.9.3

[PATCH] net: mvpp2: Fix a memory leak in error handling path

[Christophe JAILLET]

if 'devm_kzalloc()' fails, we should release resources allocated so far,
just as done a few lines below.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/ethernet/marvell/mvpp2.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c 
b/drivers/net/ethernet/marvell/mvpp2.c
index c2fd7c36f927..c48632048f71 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -5971,8 +5971,10 @@ static int mvpp2_port_init(struct mvpp2_port *port)
struct mvpp2_tx_queue *txq;
 
txq = devm_kzalloc(dev, sizeof(*txq), GFP_KERNEL);
-   if (!txq)
-   return -ENOMEM;
+   if (!txq) {
+   err = -ENOMEM;
+   goto err_free_percpu;
+   }
 
txq->pcpu = alloc_percpu(struct mvpp2_txq_pcpu);
if (!txq->pcpu) {
-- 
2.9.3

[PATCH 1/2] net: fs_enet: Fix an error handling path

[Christophe JAILLET]

'of_node_put(fpi->phy_node)' should also be called if we branch to
'out_deregister_fixed_link' error handling path.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c 
b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
index 54e3ce9bd94c..5c6426756d11 100644
--- a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
+++ b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
@@ -1045,10 +1045,10 @@ static int fs_enet_probe(struct platform_device *ofdev)
 out_free_dev:
free_netdev(ndev);
 out_put:
-   of_node_put(fpi->phy_node);
if (fpi->clk_per)
clk_disable_unprepare(fpi->clk_per);
 out_deregister_fixed_link:
+   of_node_put(fpi->phy_node);
if (of_phy_is_fixed_link(ofdev->dev.of_node))
of_phy_deregister_fixed_link(ofdev->dev.of_node);
 out_free_fpi:
-- 
2.9.3

[PATCH 2/2] net: fs_enet: Simplify code

[Christophe JAILLET]

There is no need to use an intermediate variable to handle an error code
in this case.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
I think that the remaining use of 'err' a few lines above could also be
dropped. However, it could change the return value (i.e. propagation of the
error returned by 'of_phy_register_fixed_link' instead of -ENODEV) and I'm
unsure it would be correct. So I leave it as-is.
---
 drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c 
b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
index 5c6426756d11..753259091b22 100644
--- a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
+++ b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
@@ -964,11 +964,10 @@ static int fs_enet_probe(struct platform_device *ofdev)
 */
clk = devm_clk_get(>dev, "per");
if (!IS_ERR(clk)) {
-   err = clk_prepare_enable(clk);
-   if (err) {
-   ret = err;
+   ret = clk_prepare_enable(clk);
+   if (ret)
goto out_deregister_fixed_link;
-   }
+
fpi->clk_per = clk;
}
 
-- 
2.9.3

[PATCH] bnxt_en: Fix a VXLAN vs GENEVE issue

[Christophe JAILLET]

Knowing that:
  #define TUNNEL_DST_PORT_FREE_REQ_TUNNEL_TYPE_VXLAN(0x1UL << 0)
  #define TUNNEL_DST_PORT_FREE_REQ_TUNNEL_TYPE_GENEVE   (0x5UL << 0)
and that 'bnxt_hwrm_tunnel_dst_port_alloc()' is only called with one of
these 2 constants, the TUNNEL_DST_PORT_ALLOC_REQ_TUNNEL_TYPE_GENEVE can not
trigger.

Replace the bit test that overlap by an equality test, just as in
'bnxt_hwrm_tunnel_dst_port_free()' above.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
Compile-tested only
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c 
b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index d313b02485a1..25d9ffe51825 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -3210,11 +3210,17 @@ static int bnxt_hwrm_tunnel_dst_port_alloc(struct bnxt 
*bp, __be16 port,
goto err_out;
}
 
-   if (tunnel_type & TUNNEL_DST_PORT_ALLOC_REQ_TUNNEL_TYPE_VXLAN)
+   switch (tunnel_type) {
+   case TUNNEL_DST_PORT_ALLOC_REQ_TUNNEL_TYPE_VXLAN:
bp->vxlan_fw_dst_port_id = resp->tunnel_dst_port_id;
-
-   else if (tunnel_type & TUNNEL_DST_PORT_ALLOC_REQ_TUNNEL_TYPE_GENEVE)
+   break;
+   case TUNNEL_DST_PORT_ALLOC_REQ_TUNNEL_TYPE_GENEVE:
bp->nge_fw_dst_port_id = resp->tunnel_dst_port_id;
+   break;
+   default:
+   break;
+   }
+
 err_out:
mutex_unlock(>hwrm_cmd_lock);
return rc;
-- 
2.9.3

Spurious code in commit 1bf40ada6290 ("amd-xgbe: Add support for clause 37 auto-negotiation"

[Marion & Christophe JAILLET]

Hi,

in commit 1bf40ada6290 ("amd-xgbe: Add support for clause 37 
auto-negotiation"), we can find:


diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-common.h 
b/drivers/net/ethernet/amd/xgbe/xgbe-common.h

index 695e982..8bcf4ef 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-common.h
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-common.h
   [...]
   #define XGBE_AN_CL37_PCS_MODE_MASK0x06
   #define XGBE_AN_CL37_PCS_MODE_BASEX0x00
   #define XGBE_AN_CL37_PCS_MODE_SGMII0x04
   #define XGBE_AN_CL37_TX_CONFIG_MASK0x08
   [...]

diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c 
b/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c

index d5bfbe4..723eb90 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c
   [...]
   /* Set up the Control register */
   reg = XMDIO_READ(pdata, MDIO_MMD_VEND2, MDIO_VEND2_AN_CTRL);
   reg &= XGBE_AN_CL37_TX_CONFIG_MASK;
   reg &= XGBE_AN_CL37_PCS_MODE_MASK;
   [...]

the "reg &=" statements look spurious. The 2 constants being 0x06 and 
0x08, the current code is equivalent to "reg = 0"


It is likely that "reg |=" (or "reg &= ~") was expected here.

Best regards,
CJ

[PATCH] net/mlx5: Simplify a test

[Christophe JAILLET]

'create_root_ns()' does not return an error pointer, so the test can be
simplified to be more consistent.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/ethernet/mellanox/mlx5/core/fs_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c 
b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
index 904853f9cf7a..330955f6badc 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
@@ -1833,7 +1833,7 @@ static int init_root_ns(struct mlx5_flow_steering 
*steering)
 {
 
steering->root_ns = create_root_ns(steering, FS_FT_NIC_RX);
-   if (IS_ERR_OR_NULL(steering->root_ns))
+   if (!steering->root_ns)
goto cleanup;
 
if (init_root_tree(steering, _fs, >root_ns->ns.node))
-- 
2.9.3

[PATCH] wan/fsl_ucc_hdlc: Fix size used in dma_free_coherent()

[Christophe JAILLET]

Size used with 'dma_alloc_coherent()' and 'dma_free_coherent()' should be
consistent.
Here, the size of a pointer is used in dma_alloc... and the size of the
pointed structure is used in dma_free...

This has been spotted with coccinelle, using the following script:

@r@
expression x0, x1, y0, y1, z0, z1, t0, t1, ret;
@@

*   ret = dma_alloc_coherent(x0, y0, z0, t0);
...
*   dma_free_coherent(x1, y1, ret, t1);


@script:python@
y0 << r.y0;
y1 << r.y1;

@@
if y1.find(y0) == -1:
 print "WARNING: sizes look different:  '%s'   vs   '%s'" % (y0, y1)
////

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
Untested
---
 drivers/net/wan/fsl_ucc_hdlc.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index 5fbf83d5aa57..65647533b401 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -295,11 +295,11 @@ static int uhdlc_init(struct ucc_hdlc_private *priv)
qe_muram_free(priv->ucc_pram_offset);
 free_tx_bd:
dma_free_coherent(priv->dev,
- TX_BD_RING_LEN * sizeof(struct qe_bd),
+ TX_BD_RING_LEN * sizeof(struct qe_bd *),
  priv->tx_bd_base, priv->dma_tx_bd);
 free_rx_bd:
dma_free_coherent(priv->dev,
- RX_BD_RING_LEN * sizeof(struct qe_bd),
+ RX_BD_RING_LEN * sizeof(struct qe_bd *),
  priv->rx_bd_base, priv->dma_rx_bd);
 free_uccf:
ucc_fast_free(priv->uccf);
@@ -688,7 +688,7 @@ static void uhdlc_memclean(struct ucc_hdlc_private *priv)
 
if (priv->rx_bd_base) {
dma_free_coherent(priv->dev,
- RX_BD_RING_LEN * sizeof(struct qe_bd),
+ RX_BD_RING_LEN * sizeof(struct qe_bd *),
  priv->rx_bd_base, priv->dma_rx_bd);
 
priv->rx_bd_base = NULL;
@@ -697,7 +697,7 @@ static void uhdlc_memclean(struct ucc_hdlc_private *priv)
 
if (priv->tx_bd_base) {
dma_free_coherent(priv->dev,
- TX_BD_RING_LEN * sizeof(struct qe_bd),
+ TX_BD_RING_LEN * sizeof(struct qe_bd *),
  priv->tx_bd_base, priv->dma_tx_bd);
 
priv->tx_bd_base = NULL;
-- 
2.7.4

[PATCH] ptp: Fix resource leak in case of error

[Christophe JAILLET]

A call to 'ida_simple_remove()' is missing in the error handling path.

This as been spotted with the following coccinelle script which tries to
detect missing 'ida_simple_remove()' call in error handling paths.

///
@@
expression x;
identifier l;
@@

*   x = ida_simple_get(...);
...
if (...) {
...
}
...
if (...) {
   ...
   goto l;
}
...
*   l: ... when != ida_simple_remove(...);

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
Compile-tested only
---
 drivers/ptp/ptp_clock.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index 2e481b9e8ea5..86280b7e41f3 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -263,6 +263,7 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info 
*info,
 no_device:
mutex_destroy(>tsevq_mux);
mutex_destroy(>pincfg_mux);
+   ida_simple_remove(_clocks_map, index);
 no_slot:
kfree(ptp);
 no_memory:
-- 
2.7.4

[PATCH resend] sctp: Remove some redundant code

[Christophe JAILLET]

In commit 311b21774f13 ("sctp: simplify sk_receive_queue locking"), a call
to 'skb_queue_splice_tail_init()' has been made explicit. Previously it was
hidden in 'sctp_skb_list_tail()'

Now, the code around it looks redundant. The '_init()' part of
'skb_queue_splice_tail_init()' should already do the same.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
Acked-by: Marcelo Ricardo Leitner <marcelo.leit...@gmail.com>
Acked-by: Neil Horman <nhor...@tuxdriver.com>
---
Resent because netdev@ was not in copy
Acked-by tags added
---
 net/sctp/ulpqueue.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/net/sctp/ulpqueue.c b/net/sctp/ulpqueue.c
index 877e55066f89..84d0fdaf7de9 100644
--- a/net/sctp/ulpqueue.c
+++ b/net/sctp/ulpqueue.c
@@ -140,11 +140,8 @@ int sctp_clear_pd(struct sock *sk, struct sctp_association 
*asoc)
 * we can go ahead and clear out the lobby in one shot
 */
if (!skb_queue_empty(>pd_lobby)) {
-   struct list_head *list;
skb_queue_splice_tail_init(>pd_lobby,
   >sk_receive_queue);
-   list = (struct list_head *)_sk(sk)->pd_lobby;
-   INIT_LIST_HEAD(list);
return 1;
}
} else {
-- 
2.7.4

Re: [PATCH] net: inet: diag: Fix an error handling

[Christophe JAILLET]

Le 12/09/2016 à 16:35, David Ahern a écrit :

On 9/12/16 12:02 AM, Christophe JAILLET wrote:

diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
index abfbe492ebfe..795af25cf84c 100644
--- a/net/ipv4/inet_diag.c
+++ b/net/ipv4/inet_diag.c
@@ -1134,7 +1134,6 @@ int inet_diag_handler_get_info(struct sk_buff *skb, 
struct sock *sk)
  
  	handler = inet_diag_lock_handler(sk->sk_protocol);

if (IS_ERR(handler)) {
-   inet_diag_unlock_handler(handler);
nlmsg_cancel(skb, nlh);
return PTR_ERR(handler);
}


That call is needed. inet_diag_unlock_handler does not operate on the input arg 
but a global mutex. Perhaps a better patch is to change 
inet_diag_unlock_handler() to a void.

This is obvious. Sorry for the noise.

CJ

[PATCH] net: inet: diag: Fix an error handling

[Christophe JAILLET]

If 'inet_diag_lock_handler()' returns an error, we should not call
'inet_diag_unlock_handler()' on it.
'handler' is not a valid mutexc in this case.

This has been spotted with the folowing coccinelle script:

@@
expression x;
identifier f;
@@

*   if (IS_ERR(x))
{
   ...
*  f(<+... x ...+>);
   ...
}

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 net/ipv4/inet_diag.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
index abfbe492ebfe..795af25cf84c 100644
--- a/net/ipv4/inet_diag.c
+++ b/net/ipv4/inet_diag.c
@@ -1134,7 +1134,6 @@ int inet_diag_handler_get_info(struct sk_buff *skb, 
struct sock *sk)
 
handler = inet_diag_lock_handler(sk->sk_protocol);
if (IS_ERR(handler)) {
-   inet_diag_unlock_handler(handler);
nlmsg_cancel(skb, nlh);
return PTR_ERR(handler);
}
-- 
2.7.4

[PATCH] drivers: net: phy: xgene: Fix 'remove' function

[Christophe JAILLET]

If 'IS_ERR(pdata->clk)' is true, then 'clk_disable_unprepare(pdata->clk)'
will do nothing.

It is likely that 'if (!IS_ERR(pdata->clk))' was expected here.
In fact, the test can even be removed because 'clk_disable_unprepare'
already handles such cases.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/phy/mdio-xgene.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/mdio-xgene.c b/drivers/net/phy/mdio-xgene.c
index 775674808249..92af182951be 100644
--- a/drivers/net/phy/mdio-xgene.c
+++ b/drivers/net/phy/mdio-xgene.c
@@ -424,10 +424,8 @@ static int xgene_mdio_remove(struct platform_device *pdev)
mdiobus_unregister(mdio_bus);
mdiobus_free(mdio_bus);
 
-   if (dev->of_node) {
-   if (IS_ERR(pdata->clk))
-   clk_disable_unprepare(pdata->clk);
-   }
+   if (dev->of_node)
+   clk_disable_unprepare(pdata->clk);
 
return 0;
 }
-- 
2.7.4

Re: [PATCH] RDS: Simplify code

[Christophe JAILLET]

Le 04/09/2016 à 14:20, Leon Romanovsky a écrit :

On Sat, Sep 03, 2016 at 07:33:29AM +0200, Christophe JAILLET wrote:

Calling 'list_splice' followed by 'INIT_LIST_HEAD' is equivalent to
'list_splice_init'.

It is not 100% accurate

list_splice(y, z)
INIT_LIST_HEAD(y)

==>

if (!list_empty(y))
  __list_splice(y, z, z>next);
INIT_LIST_HEAD(y)

and not

if (!list_empty(y)) {
  __list_splice(y, z, z>next);
  INIT_LIST_HEAD(y)
}

as list_splice_init will do.

You are right but if you dig further you will see that calling 
INIT_LIST_HEAD on an empty list is a no-op (AFAIK).
And if this list was not already correctly initialized, then you would 
have some other troubles.


CJ

[PATCH] RDS: Simplify code

[Christophe JAILLET]

Calling 'list_splice' followed by 'INIT_LIST_HEAD' is equivalent to
'list_splice_init'.

This has been spotted with the following coccinelle script:
/
@@
expression y,z;
@@

-   list_splice(y,z);
-   INIT_LIST_HEAD(y);
+   list_splice_init(y,z);

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 net/rds/loop.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/rds/loop.c b/net/rds/loop.c
index f2bf78de5688..c3e6da4fdf97 100644
--- a/net/rds/loop.c
+++ b/net/rds/loop.c
@@ -167,8 +167,7 @@ void rds_loop_exit(void)
 
/* avoid calling conn_destroy with irqs off */
spin_lock_irq(_conns_lock);
-   list_splice(_conns, _list);
-   INIT_LIST_HEAD(_conns);
+   list_splice_init(_conns, _list);
spin_unlock_irq(_conns_lock);
 
list_for_each_entry_safe(lc, _lc, _list, loop_node) {
-- 
2.7.4

[PATCH] mwifiex: scan: Simplify code

[Christophe JAILLET]

This patch:
   - improves code layout
   - removes a useless memset(0) for some memory allocated with kzalloc
   - removes a useless if. We know that 'if (chan_band_tlv)' will succeed
 because it has been tested a few lines above

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/wireless/marvell/mwifiex/scan.c | 12 
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c 
b/drivers/net/wireless/marvell/mwifiex/scan.c
index 21ec84794d0c..c29dd958acae 100644
--- a/drivers/net/wireless/marvell/mwifiex/scan.c
+++ b/drivers/net/wireless/marvell/mwifiex/scan.c
@@ -2179,18 +2179,14 @@ int mwifiex_ret_802_11_scan(struct mwifiex_private 
*priv,
 
if (chan_band_tlv && adapter->nd_info) {
adapter->nd_info->matches[idx] =
-   kzalloc(sizeof(*pmatch) +
-   sizeof(u32), GFP_ATOMIC);
+   kzalloc(sizeof(*pmatch) + sizeof(u32),
+   GFP_ATOMIC);
 
pmatch = adapter->nd_info->matches[idx];
 
if (pmatch) {
-   memset(pmatch, 0, sizeof(*pmatch));
-   if (chan_band_tlv) {
-   pmatch->n_channels = 1;
-   pmatch->channels[0] =
-   chan_band->chan_number;
-   }
+   pmatch->n_channels = 1;
+   pmatch->channels[0] = chan_band->chan_number;
}
}
 
-- 
2.7.4

Re: [PATCH] wan-cosa: Use memdup_user() rather than duplicating its implementation

[Christophe JAILLET]

Le 20/08/2016 à 10:25, SF Markus Elfring a écrit :

From: Markus Elfring 
Date: Sat, 20 Aug 2016 10:10:12 +0200

Reuse existing functionality from memdup_user() instead of keeping
duplicate source code.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
  drivers/net/wan/cosa.c | 12 +++-
  1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wan/cosa.c b/drivers/net/wan/cosa.c
index b87fe0a..02f5809 100644
--- a/drivers/net/wan/cosa.c
+++ b/drivers/net/wan/cosa.c
@@ -875,16 +875,10 @@ static ssize_t cosa_write(struct file *file,
if (count > COSA_MTU)
count = COSA_MTU;

-   /* Allocate the buffer */
-   kbuf = kmalloc(count, GFP_KERNEL|GFP_DMA);
In this case, 'memdup_user()' has a different meaning, as GFP_DMA will 
no more be used for this memory allocation.



-   if (kbuf == NULL) {
+   kbuf = memdup_user(buf, count);
+   if (IS_ERR(kbuf)) {
up(>wsem);
-   return -ENOMEM;
-   }
-   if (copy_from_user(kbuf, buf, count)) {
-   up(>wsem);
-   kfree(kbuf);
-   return -EFAULT;
+   return PTR_ERR(kbuf);
}
chan->tx_status=0;
cosa_start_tx(chan, kbuf, count);




---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel 
antivirus Avast.
https://www.avast.com/antivirus

[PATCH 1/2] mwifiex: fix the length parameter of a memset

[Christophe JAILLET]

In 'mwifiex_get_ver_ext', we have:
   struct mwifiex_ver_ext ver_ext;

   memset(_ext, 0, sizeof(struct host_cmd_ds_version_ext));

This is likely that memset'ing sizeof(struct mwifiex_ver_ext) was expected.
Remove the ambiguity by using the variable name directly instead of its
type.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/wireless/marvell/mwifiex/sta_ioctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c 
b/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
index e06647a..78819e8 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
@@ -1180,7 +1180,7 @@ mwifiex_get_ver_ext(struct mwifiex_private *priv, u32 
version_str_sel)
 {
struct mwifiex_ver_ext ver_ext;
 
-   memset(_ext, 0, sizeof(struct host_cmd_ds_version_ext));
+   memset(_ext, 0, sizeof(ver_ext));
ver_ext.version_str_sel = version_str_sel;
if (mwifiex_send_cmd(priv, HostCmd_CMD_VERSION_EXT,
 HostCmd_ACT_GEN_GET, 0, _ext, true))
-- 
2.7.4


---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel 
antivirus Avast.
https://www.avast.com/antivirus

[PATCH 2/2] mwifiex: simplify length computation for some memset

[Christophe JAILLET]

This patch should be a no-op. It just simplifies code by using the name of
a variable instead of its type when calling 'sizeof'.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/wireless/marvell/mwifiex/sta_ioctl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c 
b/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
index 78819e8..644f3a2 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
@@ -574,7 +574,7 @@ int mwifiex_enable_hs(struct mwifiex_adapter *adapter)
 
adapter->hs_activate_wait_q_woken = false;
 
-   memset(, 0, sizeof(struct mwifiex_ds_hs_cfg));
+   memset(, 0, sizeof(hscfg));
hscfg.is_invoke_hostcmd = true;
 
adapter->hs_enabling = true;
@@ -1138,7 +1138,7 @@ int mwifiex_set_encode(struct mwifiex_private *priv, 
struct key_params *kp,
 {
struct mwifiex_ds_encrypt_key encrypt_key;
 
-   memset(_key, 0, sizeof(struct mwifiex_ds_encrypt_key));
+   memset(_key, 0, sizeof(encrypt_key));
encrypt_key.key_len = key_len;
encrypt_key.key_index = key_index;
 
-- 
2.7.4


---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel 
antivirus Avast.
https://www.avast.com/antivirus

[PATCH] drivers: atm: nicstar: Use the correct function to free some resources

[Christophe JAILLET]

In 'get_scq', 'dma_alloc_coherent' has been used to allocate some
resources, so we need to free them using 'dma_free_coherent' instead
of 'kfree'.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/atm/nicstar.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/atm/nicstar.c b/drivers/atm/nicstar.c
index ddc4ceb..700ed15 100644
--- a/drivers/atm/nicstar.c
+++ b/drivers/atm/nicstar.c
@@ -874,7 +874,8 @@ static scq_info *get_scq(ns_dev *card, int size, u32 scd)
scq->skb = kmalloc(sizeof(struct sk_buff *) *
   (size / NS_SCQE_SIZE), GFP_KERNEL);
if (!scq->skb) {
-   kfree(scq->org);
+   dma_free_coherent(>pcidev->dev,
+ 2 * size, scq->org, scq->dma);
kfree(scq);
return NULL;
}
-- 
2.7.4


---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel 
antivirus Avast.
https://www.avast.com/antivirus

[PATCH] net: ti: cpmac: Use the correct function to free some resources.

[Christophe JAILLET]

In 'cpmac_open', 'dma_alloc_coherent' has been used to allocate some
resources, so we need to free them using 'dma_free_coherent' instead
of 'kfree'.

Also, we don't need to free these resources if the allocation has failed.
So I have slighly modified the goto label in this case.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>

---
Un-compiled (I don't have the configuration for that) and un-tested
---
 drivers/net/ethernet/ti/cpmac.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ti/cpmac.c b/drivers/net/ethernet/ti/cpmac.c
index 7eef45e..56c8920 100644
--- a/drivers/net/ethernet/ti/cpmac.c
+++ b/drivers/net/ethernet/ti/cpmac.c
@@ -1032,8 +1032,10 @@ fail_desc:
kfree_skb(priv->rx_head[i].skb);
}
}
+   dma_free_coherent(>dev, sizeof(struct cpmac_desc) * size,
+ priv->desc_ring, priv->dma_ring);
+
 fail_alloc:
-   kfree(priv->desc_ring);
iounmap(priv->regs);
 
 fail_remap:
-- 
2.7.4


---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel 
antivirus Avast.
https://www.avast.com/antivirus

[PATCH] mlxsw: spectrum_router: Return -ENOENT in case of error

[Christophe JAILLET]

'vr' should be a valid pointer here, so returning 'PTR_ERR(vr)' is wrong.
Return an explicit error code (-ENOENT) instead.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c 
b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index e084ea5..81418d6 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -1803,7 +1803,7 @@ int mlxsw_sp_router_fib4_del(struct mlxsw_sp_port 
*mlxsw_sp_port,
  sizeof(fib4->dst), fib4->dst_len);
if (!fib_entry) {
dev_warn(mlxsw_sp->bus_info->dev, "Failed to find FIB4 entry 
being removed.\n");
-   return PTR_ERR(vr);
+   return -ENOENT;
}
mlxsw_sp_fib_entry_del(mlxsw_sp_port->mlxsw_sp, fib_entry);
mlxsw_sp_fib_entry_remove(vr->fib, fib_entry);
-- 
2.7.4


---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel 
antivirus Avast.
https://www.avast.com/antivirus

[PATCH] fsl/fman: fix error handling

[Christophe JAILLET]

This is likely that checking 'fman->fifo_offset' instead of
'fman->cam_offset' is expected here.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>

---

The type of 'fifo_offset' may also need to be changed.
---
 drivers/net/ethernet/freescale/fman/fman.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/fman/fman.c 
b/drivers/net/ethernet/freescale/fman/fman.c
index 1de2e1e..f634e769 100644
--- a/drivers/net/ethernet/freescale/fman/fman.c
+++ b/drivers/net/ethernet/freescale/fman/fman.c
@@ -2036,7 +2036,7 @@ static int fman_init(struct fman *fman)
/* allocate MURAM for FIFO according to total size */
fman->fifo_offset = fman_muram_alloc(fman->muram,
 fman->state->total_fifo_size);
-   if (IS_ERR_VALUE(fman->cam_offset)) {
+   if (IS_ERR_VALUE(fman->fifo_offset)) {
free_init_resources(fman);
dev_err(fman->dev, "%s: MURAM alloc for BMI FIFO failed\n",
__func__);
-- 
2.7.4

[PATCH] net/mlx4: Fix some indent inconsistancy

[Christophe JAILLET]

Silent a few smatch warnings about indentation.
This include the removal of a 'return' statement in 'resource_tracker.c'.
This 'return' will still be performed when breaking out of the
corresponding 'switch' block.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
 drivers/net/ethernet/mellanox/mlx4/intf.c  |  2 +-
 drivers/net/ethernet/mellanox/mlx4/main.c  |  2 +-
 drivers/net/ethernet/mellanox/mlx4/mcg.c   |  8 
 drivers/net/ethernet/mellanox/mlx4/mr.c|  2 +-
 .../net/ethernet/mellanox/mlx4/resource_tracker.c  | 22 ++
 5 files changed, 17 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/intf.c 
b/drivers/net/ethernet/mellanox/mlx4/intf.c
index dec77d6..7ae1cda 100644
--- a/drivers/net/ethernet/mellanox/mlx4/intf.c
+++ b/drivers/net/ethernet/mellanox/mlx4/intf.c
@@ -147,7 +147,7 @@ int mlx4_do_bond(struct mlx4_dev *dev, bool enable)
if (enable) {
dev->flags |= MLX4_FLAG_BONDED;
} else {
-ret = mlx4_virt2phy_port_map(dev, 1, 2);
+   ret = mlx4_virt2phy_port_map(dev, 1, 2);
if (ret) {
mlx4_err(dev, "Fail to reset port map\n");
return ret;
diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c 
b/drivers/net/ethernet/mellanox/mlx4/main.c
index b673a5f..75dd2e3 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -2600,7 +2600,7 @@ static int mlx4_setup_hca(struct mlx4_dev *dev)
err = mlx4_init_uar_table(dev);
if (err) {
mlx4_err(dev, "Failed to initialize user access region table, 
aborting\n");
-return err;
+   return err;
}
 
err = mlx4_uar_alloc(dev, >driver_uar);
diff --git a/drivers/net/ethernet/mellanox/mlx4/mcg.c 
b/drivers/net/ethernet/mellanox/mlx4/mcg.c
index f2d0920..94b891c 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mcg.c
+++ b/drivers/net/ethernet/mellanox/mlx4/mcg.c
@@ -618,8 +618,8 @@ static int remove_promisc_qp(struct mlx4_dev *dev, u8 port,
err = mlx4_READ_ENTRY(dev,
  entry->index,
  mailbox);
-   if (err)
-   goto out_mailbox;
+   if (err)
+   goto out_mailbox;
members_count =
be32_to_cpu(mgm->members_count) &
0xff;
@@ -657,8 +657,8 @@ static int remove_promisc_qp(struct mlx4_dev *dev, u8 port,
err = mlx4_WRITE_ENTRY(dev,
   entry->index,
   mailbox);
-   if (err)
-   goto out_mailbox;
+   if (err)
+   goto out_mailbox;
}
}
}
diff --git a/drivers/net/ethernet/mellanox/mlx4/mr.c 
b/drivers/net/ethernet/mellanox/mlx4/mr.c
index 9319519..395b546 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mr.c
+++ b/drivers/net/ethernet/mellanox/mlx4/mr.c
@@ -248,7 +248,7 @@ static void mlx4_free_mtt_range(struct mlx4_dev *dev, u32 
offset, int order)
  offset, order);
return;
}
-__mlx4_free_mtt_range(dev, offset, order);
+   __mlx4_free_mtt_range(dev, offset, order);
 }
 
 void mlx4_mtt_cleanup(struct mlx4_dev *dev, struct mlx4_mtt *mtt)
diff --git a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c 
b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
index cd9b2b2..8b81114 100644
--- a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
+++ b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
@@ -2372,16 +2372,15 @@ static int mpt_free_res(struct mlx4_dev *dev, int 
slave, int op, int cmd,
__mlx4_mpt_release(dev, index);
break;
case RES_OP_MAP_ICM:
-   index = get_param_l(_param);
-   id = index & mpt_mask(dev);
-   err = mr_res_start_move_to(dev, slave, id,
-  RES_MPT_RESERVED, );
-   if (err)
-   return err;
-
-   __mlx4_mpt_free_icm(dev, mpt->key);
-   res_end_move(dev, slave, RES_MPT, id);
+   index = get_param_l(_param);
+   id = index & mpt_mask(dev);
+   err = mr_res_start_move_to(dev, slave, id,
+  

[PATCH] ps3_gelic: fix memcpy parameter

[Christophe JAILLET]

The size allocated for target->hwinfo and the number of bytes copied in it
should be consistent.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
---
Untested

 drivers/net/ethernet/toshiba/ps3_gelic_wireless.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_wireless.c 
b/drivers/net/ethernet/toshiba/ps3_gelic_wireless.c
index 13214a6..743b182 100644
--- a/drivers/net/ethernet/toshiba/ps3_gelic_wireless.c
+++ b/drivers/net/ethernet/toshiba/ps3_gelic_wireless.c
@@ -1622,7 +1622,7 @@ static void gelic_wl_scan_complete_event(struct 
gelic_wl_info *wl)
continue;
 
/* copy hw scan info */
-   memcpy(target->hwinfo, scan_info, scan_info->size);
+   memcpy(target->hwinfo, scan_info, be16_to_cpu(scan_info->size));
target->essid_len = strnlen(scan_info->essid,
sizeof(scan_info->essid));
target->rate_len = 0;
-- 
2.7.4

Inconsistent use of size argument in kzalloc and memcpy in 'drivers/net/ethernet/toshiba/ps3_gelic_wireless.c'

[Christophe JAILLET]

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.


I don't know which one is the correct one.


CJ

--- drivers/net/ethernet/toshiba/ps3_gelic_wireless.c
+++ /tmp/cocci-output-24201-0dddbd-ps3_gelic_wireless.c
@@ -1616,13 +1616,10 @@ static void gelic_wl_scan_complete_event
 target->valid = 1;
 target->eurus_index = i;
 kfree(target->hwinfo);
-target->hwinfo = kzalloc(be16_to_cpu(scan_info->size),
- GFP_KERNEL);
 if (!target->hwinfo)
 continue;

 /* copy hw scan info */
-memcpy(target->hwinfo, scan_info, scan_info->size);
 target->essid_len = strnlen(scan_info->essid,
 sizeof(scan_info->essid));
 target->rate_len = 0;

[PATCH] net: qlcnic: Deletion of unnecessary memset

[Christophe JAILLET]

There is no need to memset memory allocated with vzalloc.

Signed-off-by: Christophe JAILLET christophe.jail...@wanadoo.fr
---
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c 
b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
index 2f6cc42..7dbab3c 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
@@ -2403,7 +2403,6 @@ int qlcnic_alloc_tx_rings(struct qlcnic_adapter *adapter,
qlcnic_free_tx_rings(adapter);
return -ENOMEM;
}
-   memset(cmd_buf_arr, 0, TX_BUFF_RINGSIZE(tx_ring));
tx_ring-cmd_buf_arr = cmd_buf_arr;
spin_lock_init(tx_ring-tx_clean_lock);
}
-- 
2.1.4

--
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] brcmsmac: Use kstrdup to simplify code

[Christophe JAILLET]

Replace a kmalloc+strcpy by an equivalent kstrdup in order to improve
readability.

Signed-off-by: Christophe JAILLET christophe.jail...@wanadoo.fr
---
v2: fix the subject

 drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c 
b/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c
index 4813506..8a6c077 100644
--- a/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c
+++ b/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c
@@ -1476,9 +1476,7 @@ struct brcms_timer *brcms_init_timer(struct brcms_info 
*wl,
wl-timers = t;
 
 #ifdef DEBUG
-   t-name = kmalloc(strlen(name) + 1, GFP_ATOMIC);
-   if (t-name)
-   strcpy(t-name, name);
+   t-name = kstrdup(name, GFP_ATOMIC);
 #endif
 
return t;
-- 
2.1.4

--
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] mac80211: Use kstrdup to simplify code

[Christophe JAILLET]

Replace a kmalloc+strcpy by an equivalent kstrdup in order to improve
readability.

Signed-off-by: Christophe JAILLET christophe.jail...@wanadoo.fr
---
 drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c 
b/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c
index 4813506..8a6c077 100644
--- a/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c
+++ b/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c
@@ -1476,9 +1476,7 @@ struct brcms_timer *brcms_init_timer(struct brcms_info 
*wl,
wl-timers = t;
 
 #ifdef DEBUG
-   t-name = kmalloc(strlen(name) + 1, GFP_ATOMIC);
-   if (t-name)
-   strcpy(t-name, name);
+   t-name = kstrdup(name, GFP_ATOMIC);
 #endif
 
return t;
-- 
2.1.4

--
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