* [PATCH 0/3] Use refcount_t for refcount
From: Chuhong Yuan @ 2019-08-02 12:10 UTC (permalink / raw)
Cc: Leon Romanovsky, Saeed Mahameed, David S . Miller, Doug Ledford,
Jason Gunthorpe, linux-rdma, netdev, linux-kernel, Chuhong Yuan
Reference counters are preferred to use refcount_t instead of
atomic_t.
This is because the implementation of refcount_t can prevent
overflows and detect possible use-after-free.
First convert the refcount field to refcount_t in mlx5/driver.h.
Then convert the uses to refcount_() APIs.
Chuhong Yuan (3):
mlx5: Use refcount_t for refcount
net/mlx5: Use refcount_() APIs
IB/mlx5: Use refcount_() APIs
drivers/infiniband/hw/mlx5/srq_cmd.c | 6 +++---
drivers/net/ethernet/mellanox/mlx5/core/qp.c | 6 +++---
include/linux/mlx5/driver.h | 2 +-
3 files changed, 7 insertions(+), 7 deletions(-)
--
2.20.1
^ permalink raw reply
* [PATCH] net/mlx5e: Use refcount_t for refcount
From: Chuhong Yuan @ 2019-08-02 12:10 UTC (permalink / raw)
Cc: Saeed Mahameed, Leon Romanovsky, David S . Miller, netdev,
linux-rdma, linux-kernel, Chuhong Yuan
refcount_t is better for reference counters since its
implementation can prevent overflows.
So convert atomic_t ref counters to refcount_t.
Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
---
drivers/net/ethernet/mellanox/mlx5/core/lib/vxlan.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/vxlan.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/vxlan.c
index b9d4f4e19ff9..045e6564481f 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/lib/vxlan.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/vxlan.c
@@ -48,7 +48,7 @@ struct mlx5_vxlan {
struct mlx5_vxlan_port {
struct hlist_node hlist;
- atomic_t refcount;
+ refcount_t refcount;
u16 udp_port;
};
@@ -113,7 +113,7 @@ int mlx5_vxlan_add_port(struct mlx5_vxlan *vxlan, u16 port)
vxlanp = mlx5_vxlan_lookup_port(vxlan, port);
if (vxlanp) {
- atomic_inc(&vxlanp->refcount);
+ refcount_inc(&vxlanp->refcount);
return 0;
}
@@ -137,7 +137,7 @@ int mlx5_vxlan_add_port(struct mlx5_vxlan *vxlan, u16 port)
}
vxlanp->udp_port = port;
- atomic_set(&vxlanp->refcount, 1);
+ refcount_set(&vxlanp->refcount, 1);
spin_lock_bh(&vxlan->lock);
hash_add(vxlan->htable, &vxlanp->hlist, port);
@@ -170,7 +170,7 @@ int mlx5_vxlan_del_port(struct mlx5_vxlan *vxlan, u16 port)
goto out_unlock;
}
- if (atomic_dec_and_test(&vxlanp->refcount)) {
+ if (refcount_dec_and_test(&vxlanp->refcount)) {
hash_del(&vxlanp->hlist);
remove = true;
}
--
2.20.1
^ permalink raw reply related
* [PATCH] net/mlx4_core: Use refcount_t for refcount
From: Chuhong Yuan @ 2019-08-02 12:10 UTC (permalink / raw)
Cc: Tariq Toukan, David S . Miller, netdev, linux-rdma, linux-kernel,
Chuhong Yuan
refcount_t is better for reference counters since its
implementation can prevent overflows.
So convert atomic_t ref counters to refcount_t.
Also convert refcount from 0-based to 1-based.
Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
---
.../ethernet/mellanox/mlx4/resource_tracker.c | 90 +++++++++----------
1 file changed, 45 insertions(+), 45 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
index 4356f3a58002..d7e26935fd76 100644
--- a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
+++ b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
@@ -114,7 +114,7 @@ struct res_qp {
struct list_head mcg_list;
spinlock_t mcg_spl;
int local_qpn;
- atomic_t ref_count;
+ refcount_t ref_count;
u32 qpc_flags;
/* saved qp params before VST enforcement in order to restore on VGT */
u8 sched_queue;
@@ -143,7 +143,7 @@ static inline const char *mtt_states_str(enum res_mtt_states state)
struct res_mtt {
struct res_common com;
int order;
- atomic_t ref_count;
+ refcount_t ref_count;
};
enum res_mpt_states {
@@ -179,7 +179,7 @@ enum res_cq_states {
struct res_cq {
struct res_common com;
struct res_mtt *mtt;
- atomic_t ref_count;
+ refcount_t ref_count;
};
enum res_srq_states {
@@ -192,7 +192,7 @@ struct res_srq {
struct res_common com;
struct res_mtt *mtt;
struct res_cq *cq;
- atomic_t ref_count;
+ refcount_t ref_count;
};
enum res_counter_states {
@@ -1050,7 +1050,7 @@ static struct res_common *alloc_qp_tr(int id)
ret->local_qpn = id;
INIT_LIST_HEAD(&ret->mcg_list);
spin_lock_init(&ret->mcg_spl);
- atomic_set(&ret->ref_count, 0);
+ refcount_set(&ret->ref_count, 1);
return &ret->com;
}
@@ -1066,7 +1066,7 @@ static struct res_common *alloc_mtt_tr(int id, int order)
ret->com.res_id = id;
ret->order = order;
ret->com.state = RES_MTT_ALLOCATED;
- atomic_set(&ret->ref_count, 0);
+ refcount_set(&ret->ref_count, 1);
return &ret->com;
}
@@ -1110,7 +1110,7 @@ static struct res_common *alloc_cq_tr(int id)
ret->com.res_id = id;
ret->com.state = RES_CQ_ALLOCATED;
- atomic_set(&ret->ref_count, 0);
+ refcount_set(&ret->ref_count, 1);
return &ret->com;
}
@@ -1125,7 +1125,7 @@ static struct res_common *alloc_srq_tr(int id)
ret->com.res_id = id;
ret->com.state = RES_SRQ_ALLOCATED;
- atomic_set(&ret->ref_count, 0);
+ refcount_set(&ret->ref_count, 1);
return &ret->com;
}
@@ -1325,10 +1325,10 @@ static int add_res_range(struct mlx4_dev *dev, int slave, u64 base, int count,
static int remove_qp_ok(struct res_qp *res)
{
- if (res->com.state == RES_QP_BUSY || atomic_read(&res->ref_count) ||
+ if (res->com.state == RES_QP_BUSY || refcount_read(&res->ref_count) != 1 ||
!list_empty(&res->mcg_list)) {
pr_err("resource tracker: fail to remove qp, state %d, ref_count %d\n",
- res->com.state, atomic_read(&res->ref_count));
+ res->com.state, refcount_read(&res->ref_count));
return -EBUSY;
} else if (res->com.state != RES_QP_RESERVED) {
return -EPERM;
@@ -1340,11 +1340,11 @@ static int remove_qp_ok(struct res_qp *res)
static int remove_mtt_ok(struct res_mtt *res, int order)
{
if (res->com.state == RES_MTT_BUSY ||
- atomic_read(&res->ref_count)) {
+ refcount_read(&res->ref_count) != 1) {
pr_devel("%s-%d: state %s, ref_count %d\n",
__func__, __LINE__,
mtt_states_str(res->com.state),
- atomic_read(&res->ref_count));
+ refcount_read(&res->ref_count));
return -EBUSY;
} else if (res->com.state != RES_MTT_ALLOCATED)
return -EPERM;
@@ -1675,7 +1675,7 @@ static int cq_res_start_move_to(struct mlx4_dev *dev, int slave, int cqn,
} else if (state == RES_CQ_ALLOCATED) {
if (r->com.state != RES_CQ_HW)
err = -EINVAL;
- else if (atomic_read(&r->ref_count))
+ else if (refcount_read(&r->ref_count) != 1)
err = -EBUSY;
else
err = 0;
@@ -1715,7 +1715,7 @@ static int srq_res_start_move_to(struct mlx4_dev *dev, int slave, int index,
} else if (state == RES_SRQ_ALLOCATED) {
if (r->com.state != RES_SRQ_HW)
err = -EINVAL;
- else if (atomic_read(&r->ref_count))
+ else if (refcount_read(&r->ref_count) != 1)
err = -EBUSY;
} else if (state != RES_SRQ_HW || r->com.state != RES_SRQ_ALLOCATED) {
err = -EINVAL;
@@ -2808,7 +2808,7 @@ int mlx4_SW2HW_MPT_wrapper(struct mlx4_dev *dev, int slave,
goto ex_put;
if (!phys) {
- atomic_inc(&mtt->ref_count);
+ refcount_inc(&mtt->ref_count);
put_res(dev, slave, mtt->com.res_id, RES_MTT);
}
@@ -2845,7 +2845,7 @@ int mlx4_HW2SW_MPT_wrapper(struct mlx4_dev *dev, int slave,
goto ex_abort;
if (mpt->mtt)
- atomic_dec(&mpt->mtt->ref_count);
+ refcount_dec(&mpt->mtt->ref_count);
res_end_move(dev, slave, RES_MPT, id);
return 0;
@@ -3007,18 +3007,18 @@ int mlx4_RST2INIT_QP_wrapper(struct mlx4_dev *dev, int slave,
err = mlx4_DMA_wrapper(dev, slave, vhcr, inbox, outbox, cmd);
if (err)
goto ex_put_srq;
- atomic_inc(&mtt->ref_count);
+ refcount_inc(&mtt->ref_count);
qp->mtt = mtt;
- atomic_inc(&rcq->ref_count);
+ refcount_inc(&rcq->ref_count);
qp->rcq = rcq;
- atomic_inc(&scq->ref_count);
+ refcount_inc(&scq->ref_count);
qp->scq = scq;
if (scqn != rcqn)
put_res(dev, slave, scqn, RES_CQ);
if (use_srq) {
- atomic_inc(&srq->ref_count);
+ refcount_inc(&srq->ref_count);
put_res(dev, slave, srqn, RES_SRQ);
qp->srq = srq;
}
@@ -3113,7 +3113,7 @@ int mlx4_SW2HW_EQ_wrapper(struct mlx4_dev *dev, int slave,
if (err)
goto out_put;
- atomic_inc(&mtt->ref_count);
+ refcount_inc(&mtt->ref_count);
eq->mtt = mtt;
put_res(dev, slave, mtt->com.res_id, RES_MTT);
res_end_move(dev, slave, RES_EQ, res_id);
@@ -3310,7 +3310,7 @@ int mlx4_HW2SW_EQ_wrapper(struct mlx4_dev *dev, int slave,
if (err)
goto ex_put;
- atomic_dec(&eq->mtt->ref_count);
+ refcount_dec(&eq->mtt->ref_count);
put_res(dev, slave, eq->mtt->com.res_id, RES_MTT);
res_end_move(dev, slave, RES_EQ, res_id);
rem_res_range(dev, slave, res_id, 1, RES_EQ, 0);
@@ -3445,7 +3445,7 @@ int mlx4_SW2HW_CQ_wrapper(struct mlx4_dev *dev, int slave,
err = mlx4_DMA_wrapper(dev, slave, vhcr, inbox, outbox, cmd);
if (err)
goto out_put;
- atomic_inc(&mtt->ref_count);
+ refcount_inc(&mtt->ref_count);
cq->mtt = mtt;
put_res(dev, slave, mtt->com.res_id, RES_MTT);
res_end_move(dev, slave, RES_CQ, cqn);
@@ -3474,7 +3474,7 @@ int mlx4_HW2SW_CQ_wrapper(struct mlx4_dev *dev, int slave,
err = mlx4_DMA_wrapper(dev, slave, vhcr, inbox, outbox, cmd);
if (err)
goto out_move;
- atomic_dec(&cq->mtt->ref_count);
+ refcount_dec(&cq->mtt->ref_count);
res_end_move(dev, slave, RES_CQ, cqn);
return 0;
@@ -3539,9 +3539,9 @@ static int handle_resize(struct mlx4_dev *dev, int slave,
err = mlx4_DMA_wrapper(dev, slave, vhcr, inbox, outbox, cmd);
if (err)
goto ex_put1;
- atomic_dec(&orig_mtt->ref_count);
+ refcount_dec(&orig_mtt->ref_count);
put_res(dev, slave, orig_mtt->com.res_id, RES_MTT);
- atomic_inc(&mtt->ref_count);
+ refcount_inc(&mtt->ref_count);
cq->mtt = mtt;
put_res(dev, slave, mtt->com.res_id, RES_MTT);
return 0;
@@ -3627,7 +3627,7 @@ int mlx4_SW2HW_SRQ_wrapper(struct mlx4_dev *dev, int slave,
if (err)
goto ex_put_mtt;
- atomic_inc(&mtt->ref_count);
+ refcount_inc(&mtt->ref_count);
srq->mtt = mtt;
put_res(dev, slave, mtt->com.res_id, RES_MTT);
res_end_move(dev, slave, RES_SRQ, srqn);
@@ -3657,9 +3657,9 @@ int mlx4_HW2SW_SRQ_wrapper(struct mlx4_dev *dev, int slave,
err = mlx4_DMA_wrapper(dev, slave, vhcr, inbox, outbox, cmd);
if (err)
goto ex_abort;
- atomic_dec(&srq->mtt->ref_count);
+ refcount_dec(&srq->mtt->ref_count);
if (srq->cq)
- atomic_dec(&srq->cq->ref_count);
+ refcount_dec(&srq->cq->ref_count);
res_end_move(dev, slave, RES_SRQ, srqn);
return 0;
@@ -3988,11 +3988,11 @@ int mlx4_2RST_QP_wrapper(struct mlx4_dev *dev, int slave,
if (err)
goto ex_abort;
- atomic_dec(&qp->mtt->ref_count);
- atomic_dec(&qp->rcq->ref_count);
- atomic_dec(&qp->scq->ref_count);
+ refcount_dec(&qp->mtt->ref_count);
+ refcount_dec(&qp->rcq->ref_count);
+ refcount_dec(&qp->scq->ref_count);
if (qp->srq)
- atomic_dec(&qp->srq->ref_count);
+ refcount_dec(&qp->srq->ref_count);
res_end_move(dev, slave, RES_QP, qpn);
return 0;
@@ -4456,7 +4456,7 @@ int mlx4_QP_FLOW_STEERING_ATTACH_wrapper(struct mlx4_dev *dev, int slave,
if (mlx4_is_bonded(dev))
mlx4_do_mirror_rule(dev, rrule);
- atomic_inc(&rqp->ref_count);
+ refcount_inc(&rqp->ref_count);
err_put_rule:
put_res(dev, slave, vhcr->out_param, RES_FS_RULE);
@@ -4540,7 +4540,7 @@ int mlx4_QP_FLOW_STEERING_DETACH_wrapper(struct mlx4_dev *dev, int slave,
MLX4_QP_FLOW_STEERING_DETACH, MLX4_CMD_TIME_CLASS_A,
MLX4_CMD_NATIVE);
if (!err)
- atomic_dec(&rqp->ref_count);
+ refcount_dec(&rqp->ref_count);
out:
put_res(dev, slave, qpn, RES_QP);
return err;
@@ -4702,11 +4702,11 @@ static void rem_slave_qps(struct mlx4_dev *dev, int slave)
if (err)
mlx4_dbg(dev, "rem_slave_qps: failed to move slave %d qpn %d to reset\n",
slave, qp->local_qpn);
- atomic_dec(&qp->rcq->ref_count);
- atomic_dec(&qp->scq->ref_count);
- atomic_dec(&qp->mtt->ref_count);
+ refcount_dec(&qp->rcq->ref_count);
+ refcount_dec(&qp->scq->ref_count);
+ refcount_dec(&qp->mtt->ref_count);
if (qp->srq)
- atomic_dec(&qp->srq->ref_count);
+ refcount_dec(&qp->srq->ref_count);
state = RES_QP_MAPPED;
break;
default:
@@ -4768,9 +4768,9 @@ static void rem_slave_srqs(struct mlx4_dev *dev, int slave)
mlx4_dbg(dev, "rem_slave_srqs: failed to move slave %d srq %d to SW ownership\n",
slave, srqn);
- atomic_dec(&srq->mtt->ref_count);
+ refcount_dec(&srq->mtt->ref_count);
if (srq->cq)
- atomic_dec(&srq->cq->ref_count);
+ refcount_dec(&srq->cq->ref_count);
state = RES_SRQ_ALLOCATED;
break;
@@ -4805,7 +4805,7 @@ static void rem_slave_cqs(struct mlx4_dev *dev, int slave)
spin_lock_irq(mlx4_tlock(dev));
list_for_each_entry_safe(cq, tmp, cq_list, com.list) {
spin_unlock_irq(mlx4_tlock(dev));
- if (cq->com.owner == slave && !atomic_read(&cq->ref_count)) {
+ if (cq->com.owner == slave && refcount_read(&cq->ref_count) == 1) {
cqn = cq->com.res_id;
state = cq->com.from_state;
while (state != 0) {
@@ -4832,7 +4832,7 @@ static void rem_slave_cqs(struct mlx4_dev *dev, int slave)
if (err)
mlx4_dbg(dev, "rem_slave_cqs: failed to move slave %d cq %d to SW ownership\n",
slave, cqn);
- atomic_dec(&cq->mtt->ref_count);
+ refcount_dec(&cq->mtt->ref_count);
state = RES_CQ_ALLOCATED;
break;
@@ -4900,7 +4900,7 @@ static void rem_slave_mrs(struct mlx4_dev *dev, int slave)
mlx4_dbg(dev, "rem_slave_mrs: failed to move slave %d mpt %d to SW ownership\n",
slave, mptn);
if (mpt->mtt)
- atomic_dec(&mpt->mtt->ref_count);
+ refcount_dec(&mpt->mtt->ref_count);
state = RES_MPT_MAPPED;
break;
default:
@@ -5144,7 +5144,7 @@ static void rem_slave_eqs(struct mlx4_dev *dev, int slave)
if (err)
mlx4_dbg(dev, "rem_slave_eqs: failed to move slave %d eqs %d to SW ownership\n",
slave, eqn & 0x3ff);
- atomic_dec(&eq->mtt->ref_count);
+ refcount_dec(&eq->mtt->ref_count);
state = RES_EQ_RESERVED;
break;
--
2.20.1
^ permalink raw reply related
* Re: [PATCH -next] iwlwifi: dbg: work around clang bug by marking debug strings static
From: Michael Ellerman @ 2019-08-02 12:04 UTC (permalink / raw)
To: Johannes Berg, Nick Desaulniers, kvalo, Luca Coelho
Cc: Arnd Bergmann, Nathan Chancellor, Emmanuel Grumbach,
Intel Linux Wireless, David S. Miller, Shahar S Matityahu,
Sara Sharon, linux-wireless, netdev, linux-kernel,
clang-built-linux
In-Reply-To: <3a2b6d4f9356d54ab8e83fbf25ba9c5f50181f0d.camel@sipsolutions.net>
Johannes Berg <johannes@sipsolutions.net> writes:
>> Luca, you said this was already fixed in your internal tree, and the fix
>> would appear soon in next, but I don't see anything in linux-next?
>
> Luca is still on vacation, but I just sent out a version of the patch we
> had applied internally.
Awesome, thanks.
cheers
^ permalink raw reply
* [PATCH 2/4] can: peak_usb: force the string buffer NULL-terminated
From: Marc Kleine-Budde @ 2019-08-02 12:00 UTC (permalink / raw)
To: netdev; +Cc: davem, linux-can, kernel, Wang Xiayang, Marc Kleine-Budde
In-Reply-To: <20190802120038.18154-1-mkl@pengutronix.de>
From: Wang Xiayang <xywang.sjtu@sjtu.edu.cn>
strncpy() does not ensure NULL-termination when the input string size
equals to the destination buffer size IFNAMSIZ. The output string is
passed to dev_info() which relies on the NULL-termination.
Use strlcpy() instead.
This issue is identified by a Coccinelle script.
Signed-off-by: Wang Xiayang <xywang.sjtu@sjtu.edu.cn>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/usb/peak_usb/pcan_usb_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_core.c b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
index 22b9c8e6d040..65dce642b86b 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_core.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
@@ -855,7 +855,7 @@ static void peak_usb_disconnect(struct usb_interface *intf)
dev_prev_siblings = dev->prev_siblings;
dev->state &= ~PCAN_USB_STATE_CONNECTED;
- strncpy(name, netdev->name, IFNAMSIZ);
+ strlcpy(name, netdev->name, IFNAMSIZ);
unregister_netdev(netdev);
--
2.20.1
^ permalink raw reply related
* [PATCH 1/4] can: sja1000: force the string buffer NULL-terminated
From: Marc Kleine-Budde @ 2019-08-02 12:00 UTC (permalink / raw)
To: netdev; +Cc: davem, linux-can, kernel, Wang Xiayang, Marc Kleine-Budde
In-Reply-To: <20190802120038.18154-1-mkl@pengutronix.de>
From: Wang Xiayang <xywang.sjtu@sjtu.edu.cn>
strncpy() does not ensure NULL-termination when the input string size
equals to the destination buffer size IFNAMSIZ. The output string
'name' is passed to dev_info which relies on NULL-termination.
Use strlcpy() instead.
This issue is identified by a Coccinelle script.
Signed-off-by: Wang Xiayang <xywang.sjtu@sjtu.edu.cn>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/sja1000/peak_pcmcia.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/can/sja1000/peak_pcmcia.c b/drivers/net/can/sja1000/peak_pcmcia.c
index 185c7f7d38a4..5e0d5e8101c8 100644
--- a/drivers/net/can/sja1000/peak_pcmcia.c
+++ b/drivers/net/can/sja1000/peak_pcmcia.c
@@ -479,7 +479,7 @@ static void pcan_free_channels(struct pcan_pccard *card)
if (!netdev)
continue;
- strncpy(name, netdev->name, IFNAMSIZ);
+ strlcpy(name, netdev->name, IFNAMSIZ);
unregister_sja1000dev(netdev);
--
2.20.1
^ permalink raw reply related
* [PATCH 4/4] can: peak_usb: pcan_usb_pro: Fix info-leaks to USB devices
From: Marc Kleine-Budde @ 2019-08-02 12:00 UTC (permalink / raw)
To: netdev
Cc: davem, linux-can, kernel, Tomas Bortoli,
syzbot+d6a5a1a3657b596ef132, linux-stable, Marc Kleine-Budde
In-Reply-To: <20190802120038.18154-1-mkl@pengutronix.de>
From: Tomas Bortoli <tomasbortoli@gmail.com>
Uninitialized Kernel memory can leak to USB devices.
Fix by using kzalloc() instead of kmalloc() on the affected buffers.
Signed-off-by: Tomas Bortoli <tomasbortoli@gmail.com>
Reported-by: syzbot+d6a5a1a3657b596ef132@syzkaller.appspotmail.com
Fixes: f14e22435a27 ("net: can: peak_usb: Do not do dma on the stack")
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/usb/peak_usb/pcan_usb_pro.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_pro.c b/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
index 178bb7cff0c1..53cb2f72bdd0 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
@@ -494,7 +494,7 @@ static int pcan_usb_pro_drv_loaded(struct peak_usb_device *dev, int loaded)
u8 *buffer;
int err;
- buffer = kmalloc(PCAN_USBPRO_FCT_DRVLD_REQ_LEN, GFP_KERNEL);
+ buffer = kzalloc(PCAN_USBPRO_FCT_DRVLD_REQ_LEN, GFP_KERNEL);
if (!buffer)
return -ENOMEM;
--
2.20.1
^ permalink raw reply related
* [PATCH 3/4] can: peak_usb: pcan_usb_fd: uix info-leaks to USB devices
From: Marc Kleine-Budde @ 2019-08-02 12:00 UTC (permalink / raw)
To: netdev
Cc: davem, linux-can, kernel, Tomas Bortoli,
syzbot+513e4d0985298538bf9b, linux-stable, Marc Kleine-Budde
In-Reply-To: <20190802120038.18154-1-mkl@pengutronix.de>
From: Tomas Bortoli <tomasbortoli@gmail.com>
Uninitialized Kernel memory can leak to USB devices.
Fix by using kzalloc() instead of kmalloc() on the affected buffers.
Signed-off-by: Tomas Bortoli <tomasbortoli@gmail.com>
Reported-by: syzbot+513e4d0985298538bf9b@syzkaller.appspotmail.com
Fixes: 0a25e1f4f185 ("can: peak_usb: add support for PEAK new CANFD USB adapters")
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/usb/peak_usb/pcan_usb_fd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
index 34761c3a6286..47cc1ff5b88e 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
@@ -841,7 +841,7 @@ static int pcan_usb_fd_init(struct peak_usb_device *dev)
goto err_out;
/* allocate command buffer once for all for the interface */
- pdev->cmd_buffer_addr = kmalloc(PCAN_UFD_CMD_BUFFER_SIZE,
+ pdev->cmd_buffer_addr = kzalloc(PCAN_UFD_CMD_BUFFER_SIZE,
GFP_KERNEL);
if (!pdev->cmd_buffer_addr)
goto err_out_1;
--
2.20.1
^ permalink raw reply related
* pull-request: can 2019-08-02
From: Marc Kleine-Budde @ 2019-08-02 12:00 UTC (permalink / raw)
To: netdev; +Cc: davem, linux-can, kernel
Hello David,
this is a pull request of 4 patches for net/master.
The first two patches are by Wang Xiayang, they force that the string buffer
during a dev_info() is properly NULL terminated.
The last two patches are by Tomas Bortoli and fix both a potential info leak of
kernel memory to USB devices.
regards,
Marc
---
The following changes since commit 224c04973db1125fcebefffd86115f99f50f8277:
net: usb: pegasus: fix improper read if get_registers() fail (2019-08-01 18:18:27 -0400)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git tags/linux-can-fixes-for-5.3-20190802
for you to fetch changes up to ead16e53c2f0ed946d82d4037c630e2f60f4ab69:
can: peak_usb: pcan_usb_pro: Fix info-leaks to USB devices (2019-08-02 13:58:01 +0200)
----------------------------------------------------------------
linux-can-fixes-for-5.3-20190802
----------------------------------------------------------------
Tomas Bortoli (2):
can: peak_usb: pcan_usb_fd: Fix info-leaks to USB devices
can: peak_usb: pcan_usb_pro: Fix info-leaks to USB devices
Wang Xiayang (2):
can: sja1000: force the string buffer NULL-terminated
can: peak_usb: force the string buffer NULL-terminated
drivers/net/can/sja1000/peak_pcmcia.c | 2 +-
drivers/net/can/usb/peak_usb/pcan_usb_core.c | 2 +-
drivers/net/can/usb/peak_usb/pcan_usb_fd.c | 2 +-
drivers/net/can/usb/peak_usb/pcan_usb_pro.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
^ permalink raw reply
* Re: [PATCH] peak_usb: Fix info-leaks to USB devices
From: Marc Kleine-Budde @ 2019-08-02 11:45 UTC (permalink / raw)
To: Tomas Bortoli, wg, linux-can
Cc: davem, gregkh, alexios.zavras, tglx, allison, netdev,
linux-kernel, syzkaller
In-Reply-To: <20190731145447.29270-1-tomasbortoli@gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 766 bytes --]
On 7/31/19 4:54 PM, Tomas Bortoli wrote:
> Uninitialized Kernel memory can leak to USB devices.
>
> Fix by using kzalloc() instead of kmalloc() on the affected buffers.
>
> Signed-off-by: Tomas Bortoli <tomasbortoli@gmail.com>
> Reported-by: syzbot+d6a5a1a3657b596ef132@syzkaller.appspotmail.com
> Reported-by: syzbot+513e4d0985298538bf9b@syzkaller.appspotmail.com
Applied, but split into two patches, as the problem was introduced in
two separate commits.
regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* RE: [PATCH][net-next] net/mlx5: remove self-assignment on esw->dev
From: Parav Pandit @ 2019-08-02 11:31 UTC (permalink / raw)
To: Colin King, Saeed Mahameed, Leon Romanovsky, David S . Miller,
netdev@vger.kernel.org, linux-rdma@vger.kernel.org
Cc: kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20190802102223.26198-1-colin.king@canonical.com>
> -----Original Message-----
> From: linux-rdma-owner@vger.kernel.org <linux-rdma-
> owner@vger.kernel.org> On Behalf Of Colin King
> Sent: Friday, August 2, 2019 3:52 PM
> To: Saeed Mahameed <saeedm@mellanox.com>; Leon Romanovsky
> <leon@kernel.org>; David S . Miller <davem@davemloft.net>;
> netdev@vger.kernel.org; linux-rdma@vger.kernel.org
> Cc: kernel-janitors@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: [PATCH][net-next] net/mlx5: remove self-assignment on esw->dev
>
> From: Colin Ian King <colin.king@canonical.com>
>
> There is a self assignment of esw->dev to itself, clean this up by removing it.
>
> Addresses-Coverity: ("Self assignment")
> Fixes: 6cedde451399 ("net/mlx5: E-Switch, Verify support QoS element type")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/net/ethernet/mellanox/mlx5/core/eswitch.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> index f4ace5f8e884..de0894b695e3 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> @@ -1413,7 +1413,7 @@ static int esw_vport_egress_config(struct
> mlx5_eswitch *esw,
>
> static bool element_type_supported(struct mlx5_eswitch *esw, int type) {
Making it const struct mlx5_eswitch *esw brings improves code hygiene further in such functions.
> - struct mlx5_core_dev *dev = esw->dev = esw->dev;
> + struct mlx5_core_dev *dev = esw->dev;
>
> switch (type) {
> case SCHEDULING_CONTEXT_ELEMENT_TYPE_TSAR:
> --
> 2.20.1
^ permalink raw reply
* Re: [PATCH 05/14] gpio: lpc32xx: allow building on non-lpc32xx targets
From: Arnd Bergmann @ 2019-08-02 11:19 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: soc, arm-soc, Vladimir Zapolskiy, Sylvain Lemieux, Russell King,
Gregory Clement, Linus Walleij, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, David S. Miller, Greg Kroah-Hartman,
Alan Stern, Guenter Roeck, linux-gpio, netdev, linux-serial,
USB list, LINUXWATCHDOG, Lee Jones, LKML
In-Reply-To: <CAMpxmJWFfT_vrDas2fzW5tnxskk9kmgHQpGnGQ-_C20UaS_jhA@mail.gmail.com>
On Fri, Aug 2, 2019 at 9:10 AM Bartosz Golaszewski
<bgolaszewski@baylibre.com> wrote:
> > -#include <mach/hardware.h>
> > -#include <mach/platform.h>
> > +#define _GPREG(x) (x)
>
> What purpose does this macro serve?
>
> >
> > #define LPC32XX_GPIO_P3_INP_STATE _GPREG(0x000)
> > #define LPC32XX_GPIO_P3_OUTP_SET _GPREG(0x004)
In the existing code base, this macro converts a register offset to
an __iomem pointer for a gpio register. I changed the definition of the
macro here to keep the number of changes down, but I it's just
as easy to remove it if you prefer.
> > @@ -167,14 +166,26 @@ struct lpc32xx_gpio_chip {
> > struct gpio_regs *gpio_grp;
> > };
> >
> > +void __iomem *gpio_reg_base;
>
> Any reason why this can't be made part of struct lpc32xx_gpio_chip?
It could be, but it's the same for each instance, and not known until
probe() time, so the same pointer would need to be copied into each
instance that is otherwise read-only.
Let me know if you'd prefer me to rework these two things or leave
them as they are.
> > +static inline u32 gpreg_read(unsigned long offset)
>
> Here and elsewhere: could you please keep the lpc32xx_gpio prefix for
> all symbols?
Sure.
Arnd
^ permalink raw reply
* Re: [PATCH iproute2-next] ip tunnel: add json output
From: Andrea Claudi @ 2019-08-02 11:14 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: linux-netdev, David Ahern
In-Reply-To: <20190801081620.6b25d23c@hermes.lan>
On Thu, Aug 1, 2019 at 5:16 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Thu, 1 Aug 2019 12:12:58 +0200
> Andrea Claudi <aclaudi@redhat.com> wrote:
>
> > Add json support on iptunnel and ip6tunnel.
> > The plain text output format should remain the same.
> >
> > Signed-off-by: Andrea Claudi <aclaudi@redhat.com>
> > ---
> > ip/ip6tunnel.c | 82 +++++++++++++++++++++++++++++++--------------
> > ip/iptunnel.c | 90 +++++++++++++++++++++++++++++++++-----------------
> > ip/tunnel.c | 42 +++++++++++++++++------
> > 3 files changed, 148 insertions(+), 66 deletions(-)
> >
> > diff --git a/ip/ip6tunnel.c b/ip/ip6tunnel.c
> > index d7684a673fdc4..f2b9710c1320f 100644
> > --- a/ip/ip6tunnel.c
> > +++ b/ip/ip6tunnel.c
> > @@ -71,57 +71,90 @@ static void usage(void)
> > static void print_tunnel(const void *t)
> > {
> > const struct ip6_tnl_parm2 *p = t;
> > - char s1[1024];
> > - char s2[1024];
> > + SPRINT_BUF(b1);
> >
> > /* Do not use format_host() for local addr,
> > * symbolic name will not be useful.
> > */
> > - printf("%s: %s/ipv6 remote %s local %s",
> > - p->name,
> > - tnl_strproto(p->proto),
> > - format_host_r(AF_INET6, 16, &p->raddr, s1, sizeof(s1)),
> > - rt_addr_n2a_r(AF_INET6, 16, &p->laddr, s2, sizeof(s2)));
> > + open_json_object(NULL);
> > + print_string(PRINT_ANY, "ifname", "%s: ", p->name);
>
> Print this using color for interface name?
Thanks for the suggestion, I can do the same also for remote and local
addresses?
>
>
> > + snprintf(b1, sizeof(b1), "%s/ipv6", tnl_strproto(p->proto));
> > + print_string(PRINT_ANY, "mode", "%s ", b1);
> > + print_string(PRINT_ANY,
> > + "remote",
> > + "remote %s ",
> > + format_host_r(AF_INET6, 16, &p->raddr, b1, sizeof(b1)));
> > + print_string(PRINT_ANY,
> > + "local",
> > + "local %s",
> > + rt_addr_n2a_r(AF_INET6, 16, &p->laddr, b1, sizeof(b1)));
> > +
> > if (p->link) {
> > const char *n = ll_index_to_name(p->link);
> >
> > if (n)
> > - printf(" dev %s", n);
> > + print_string(PRINT_ANY, "link", " dev %s", n);
> > }
> >
> > if (p->flags & IP6_TNL_F_IGN_ENCAP_LIMIT)
> > - printf(" encaplimit none");
> > + print_bool(PRINT_ANY,
> > + "ip6_tnl_f_ign_encap_limit",
> > + " encaplimit none",
> > + true);
>
> For flags like this, print_null is more typical JSON than a boolean
> value. Null is better for presence flag. Bool is better if both true and
> false are printed.
Using print_null json JSON output becomes:
{
"ifname": "gre2",
"mode": "gre/ipv6",
"remote": "fd::1",
"local": "::",
"ip6_tnl_f_ign_encap_limit": null,
"hoplimit": 64,
"tclass": "0x00",
"flowlabel": "0x00000",
"flowinfo": "0x00000000",
"flags": []
}
Which seems a bit confusing to me (what does the "null" value means?).
Using print_null we also introduce an inconsistency with the JSON
output of ip/link_gre6.c and ip/link_ip6tnl.c.
I would prefer to use print_bool and print out
ip6_tnl_f_ign_encap_limit both when true and false, patching both
files above to do the same. WDYT?
^ permalink raw reply
* Re: [PATCH net 0/2] flow_offload hardware priority fixes
From: Pablo Neira Ayuso @ 2019-08-02 11:00 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netfilter-devel, davem, netdev, marcelo.leitner, jiri, wenxu,
saeedm, paulb, gerlitz.or
In-Reply-To: <20190801172014.314a9d01@cakuba.netronome.com>
Hi Jakub,
If the user specifies 'pref' in the new rule, then tc checks if there
is a tcf_proto object that matches this priority. If the tcf_proto
object does not exist, tc creates a tcf_proto object and it adds the
new rule to this tcf_proto.
In cls_flower, each tcf_proto only stores one single rule, so if the
user tries to add another rule with the same 'pref', cls_flower
returns EEXIST.
I'll prepare a new patchset not to map the priority to the netfilter
basechain priority, instead the rule priority will be internally
allocated for each new rule.
Thanks for your feedback.
^ permalink raw reply
* [PATCH net v4] net: bridge: move default pvid init/deinit to NETDEV_REGISTER/UNREGISTER
From: Nikolay Aleksandrov @ 2019-08-02 10:57 UTC (permalink / raw)
To: netdev; +Cc: roopa, davem, bridge, Nikolay Aleksandrov, michael-dev
In-Reply-To: <0a015a21-c1ae-e275-e675-431f08bece86@cumulusnetworks.com>
Most of the bridge device's vlan init bugs come from the fact that its
default pvid is created at the wrong time, way too early in ndo_init()
before the device is even assigned an ifindex. It introduces a bug when the
bridge's dev_addr is added as fdb during the initial default pvid creation
the notification has ifindex/NDA_MASTER both equal to 0 (see example below)
which really makes no sense for user-space[0] and is wrong.
Usually user-space software would ignore such entries, but they are
actually valid and will eventually have all necessary attributes.
It makes much more sense to send a notification *after* the device has
registered and has a proper ifindex allocated rather than before when
there's a chance that the registration might still fail or to receive
it with ifindex/NDA_MASTER == 0. Note that we can remove the fdb flush
from br_vlan_flush() since that case can no longer happen. At
NETDEV_REGISTER br->default_pvid is always == 1 as it's initialized by
br_vlan_init() before that and at NETDEV_UNREGISTER it can be anything
depending why it was called (if called due to NETDEV_REGISTER error
it'll still be == 1, otherwise it could be any value changed during the
device life time).
For the demonstration below a small change to iproute2 for printing all fdb
notifications is added, because it contained a workaround not to show
entries with ifindex == 0.
Command executed while monitoring: $ ip l add br0 type bridge
Before (both ifindex and master == 0):
$ bridge monitor fdb
36:7e:8a:b3:56:ba dev * vlan 1 master * permanent
After (proper br0 ifindex):
$ bridge monitor fdb
e6:2a:ae:7a:b7:48 dev br0 vlan 1 master br0 permanent
v4: move only the default pvid init/deinit to NETDEV_REGISTER/UNREGISTER
v3: send the correct v2 patch with all changes (stub should return 0)
v2: on error in br_vlan_init set br->vlgrp to NULL and return 0 in
the br_vlan_bridge_event stub when bridge vlans are disabled
[0] https://bugzilla.kernel.org/show_bug.cgi?id=204389
Reported-by: michael-dev <michael-dev@fami-braun.de>
Fixes: 5be5a2df40f0 ("bridge: Add filtering support for default_pvid")
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
To test the patch I've added manual error conditions all over the bridge
init process to exercise all of the error handling paths.
This change is equivalent to v3 w.r.t. init ordering fixes, the core
of the problem was the default pvid init/deinit sequences being done
at wrong times, so we delay them until NETDEV_REGISTER/UNREGISTER.
net/bridge/br.c | 5 ++++-
net/bridge/br_private.h | 9 +++++----
net/bridge/br_vlan.c | 34 ++++++++++++++++------------------
3 files changed, 25 insertions(+), 23 deletions(-)
diff --git a/net/bridge/br.c b/net/bridge/br.c
index d164f63a4345..8a8f9e5f264f 100644
--- a/net/bridge/br.c
+++ b/net/bridge/br.c
@@ -37,12 +37,15 @@ static int br_device_event(struct notifier_block *unused, unsigned long event, v
int err;
if (dev->priv_flags & IFF_EBRIDGE) {
+ err = br_vlan_bridge_event(dev, event, ptr);
+ if (err)
+ return notifier_from_errno(err);
+
if (event == NETDEV_REGISTER) {
/* register of bridge completed, add sysfs entries */
br_sysfs_addbr(dev);
return NOTIFY_DONE;
}
- br_vlan_bridge_event(dev, event, ptr);
}
/* not a port of a bridge */
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index e8cf03b43b7d..646504db0220 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -894,8 +894,8 @@ int nbp_get_num_vlan_infos(struct net_bridge_port *p, u32 filter_mask);
void br_vlan_get_stats(const struct net_bridge_vlan *v,
struct br_vlan_stats *stats);
void br_vlan_port_event(struct net_bridge_port *p, unsigned long event);
-void br_vlan_bridge_event(struct net_device *dev, unsigned long event,
- void *ptr);
+int br_vlan_bridge_event(struct net_device *dev, unsigned long event,
+ void *ptr);
static inline struct net_bridge_vlan_group *br_vlan_group(
const struct net_bridge *br)
@@ -1085,9 +1085,10 @@ static inline void br_vlan_port_event(struct net_bridge_port *p,
{
}
-static inline void br_vlan_bridge_event(struct net_device *dev,
- unsigned long event, void *ptr)
+static inline int br_vlan_bridge_event(struct net_device *dev,
+ unsigned long event, void *ptr)
{
+ return 0;
}
#endif
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index a544e161c7fa..f5b2aeebbfe9 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -715,11 +715,6 @@ void br_vlan_flush(struct net_bridge *br)
ASSERT_RTNL();
- /* delete auto-added default pvid local fdb before flushing vlans
- * otherwise it will be leaked on bridge device init failure
- */
- br_fdb_delete_by_port(br, NULL, 0, 1);
-
vg = br_vlan_group(br);
__vlan_flush(vg);
RCU_INIT_POINTER(br->vlgrp, NULL);
@@ -1058,7 +1053,6 @@ int br_vlan_init(struct net_bridge *br)
{
struct net_bridge_vlan_group *vg;
int ret = -ENOMEM;
- bool changed;
vg = kzalloc(sizeof(*vg), GFP_KERNEL);
if (!vg)
@@ -1073,17 +1067,10 @@ int br_vlan_init(struct net_bridge *br)
br->vlan_proto = htons(ETH_P_8021Q);
br->default_pvid = 1;
rcu_assign_pointer(br->vlgrp, vg);
- ret = br_vlan_add(br, 1,
- BRIDGE_VLAN_INFO_PVID | BRIDGE_VLAN_INFO_UNTAGGED |
- BRIDGE_VLAN_INFO_BRENTRY, &changed, NULL);
- if (ret)
- goto err_vlan_add;
out:
return ret;
-err_vlan_add:
- vlan_tunnel_deinit(vg);
err_tunnel_init:
rhashtable_destroy(&vg->vlan_hash);
err_rhtbl:
@@ -1469,13 +1456,23 @@ static void nbp_vlan_set_vlan_dev_state(struct net_bridge_port *p, u16 vid)
}
/* Must be protected by RTNL. */
-void br_vlan_bridge_event(struct net_device *dev, unsigned long event,
- void *ptr)
+int br_vlan_bridge_event(struct net_device *dev, unsigned long event, void *ptr)
{
struct netdev_notifier_changeupper_info *info;
- struct net_bridge *br;
+ struct net_bridge *br = netdev_priv(dev);
+ bool changed;
+ int ret = 0;
switch (event) {
+ case NETDEV_REGISTER:
+ ret = br_vlan_add(br, br->default_pvid,
+ BRIDGE_VLAN_INFO_PVID |
+ BRIDGE_VLAN_INFO_UNTAGGED |
+ BRIDGE_VLAN_INFO_BRENTRY, &changed, NULL);
+ break;
+ case NETDEV_UNREGISTER:
+ br_vlan_delete(br, br->default_pvid);
+ break;
case NETDEV_CHANGEUPPER:
info = ptr;
br_vlan_upper_change(dev, info->upper_dev, info->linking);
@@ -1483,12 +1480,13 @@ void br_vlan_bridge_event(struct net_device *dev, unsigned long event,
case NETDEV_CHANGE:
case NETDEV_UP:
- br = netdev_priv(dev);
if (!br_opt_get(br, BROPT_VLAN_BRIDGE_BINDING))
- return;
+ break;
br_vlan_link_state_change(dev, br);
break;
}
+
+ return ret;
}
/* Must be protected by RTNL. */
--
2.21.0
^ permalink raw reply related
* [PATCH 2/2] ixgbe: Use refcount_t for refcount
From: Chuhong Yuan @ 2019-08-02 10:55 UTC (permalink / raw)
Cc: Jeff Kirsher, David S . Miller, intel-wired-lan, netdev,
linux-kernel, Chuhong Yuan
refcount_t is better for reference counters since its
implementation can prevent overflows.
So convert atomic_t ref counters to refcount_t.
Also convert refcount from 0-based to 1-based.
This patch depends on PATCH 1/2.
Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c | 6 +++---
drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.h | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
index 00710f43cfd2..d313d00065cd 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
@@ -773,7 +773,7 @@ int ixgbe_setup_fcoe_ddp_resources(struct ixgbe_adapter *adapter)
fcoe->extra_ddp_buffer = buffer;
fcoe->extra_ddp_buffer_dma = dma;
- atomic_set(&fcoe->refcnt, 0);
+ refcount_set(&fcoe->refcnt, 1);
/* allocate pci pool for each cpu */
for_each_possible_cpu(cpu) {
@@ -837,7 +837,7 @@ int ixgbe_fcoe_enable(struct net_device *netdev)
struct ixgbe_adapter *adapter = netdev_priv(netdev);
struct ixgbe_fcoe *fcoe = &adapter->fcoe;
- atomic_inc(&fcoe->refcnt);
+ refcount_inc(&fcoe->refcnt);
if (!(adapter->flags & IXGBE_FLAG_FCOE_CAPABLE))
return -EINVAL;
@@ -883,7 +883,7 @@ int ixgbe_fcoe_disable(struct net_device *netdev)
{
struct ixgbe_adapter *adapter = netdev_priv(netdev);
- if (!atomic_dec_and_test(&adapter->fcoe.refcnt))
+ if (!refcount_dec_and_test(&adapter->fcoe.refcnt))
return -EINVAL;
if (!(adapter->flags & IXGBE_FLAG_FCOE_ENABLED))
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.h
index 724f5382329f..7ace5fee6ede 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.h
@@ -51,7 +51,7 @@ struct ixgbe_fcoe_ddp_pool {
struct ixgbe_fcoe {
struct ixgbe_fcoe_ddp_pool __percpu *ddp_pool;
- atomic_t refcnt;
+ refcount_t refcnt;
spinlock_t lock;
struct ixgbe_fcoe_ddp ddp[IXGBE_FCOE_DDP_MAX_X550];
void *extra_ddp_buffer;
--
2.20.1
^ permalink raw reply related
* [PATCH 1/2] ixgbe: Explicitly initialize reference count to 0
From: Chuhong Yuan @ 2019-08-02 10:54 UTC (permalink / raw)
Cc: Jeff Kirsher, David S . Miller, intel-wired-lan, netdev,
linux-kernel, Chuhong Yuan
The driver does not explicitly call atomic_set to initialize
refcount to 0.
Add the call so that it will be more straight forward to
convert refcount from atomic_t to refcount_t.
Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
index ccd852ad62a4..00710f43cfd2 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
@@ -773,6 +773,7 @@ int ixgbe_setup_fcoe_ddp_resources(struct ixgbe_adapter *adapter)
fcoe->extra_ddp_buffer = buffer;
fcoe->extra_ddp_buffer_dma = dma;
+ atomic_set(&fcoe->refcnt, 0);
/* allocate pci pool for each cpu */
for_each_possible_cpu(cpu) {
--
2.20.1
^ permalink raw reply related
* Re: [PATCH net-next v5 5/6] flow_offload: support get flow_block immediately
From: wenxu @ 2019-08-02 10:45 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: jiri, pablo, fw, netfilter-devel, netdev, John Hurley
In-Reply-To: <20190801161129.25fee619@cakuba.netronome.com>
On 8/2/2019 7:11 AM, Jakub Kicinski wrote:
> On Thu, 1 Aug 2019 11:03:46 +0800, wenxu@ucloud.cn wrote:
>> From: wenxu <wenxu@ucloud.cn>
>>
>> The new flow-indr-block can't get the tcf_block
>> directly. It provide a callback list to find the flow_block immediately
>> when the device register and contain a ingress block.
>>
>> Signed-off-by: wenxu <wenxu@ucloud.cn>
> First of all thanks for splitting the series up into more patches,
> it is easier to follow the logic now!
>
>> @@ -328,6 +348,7 @@ struct flow_indr_block_dev {
>>
>> INIT_LIST_HEAD(&indr_dev->cb_list);
>> indr_dev->dev = dev;
>> + flow_get_default_block(indr_dev);
>> if (rhashtable_insert_fast(&indr_setup_block_ht, &indr_dev->ht_node,
>> flow_indr_setup_block_ht_params)) {
>> kfree(indr_dev);
> I wonder if it's still practical to keep the block information in the
> indr_dev structure at all. The way this used to work was:
>
>
> [hash table of devices] --------------
> | | netdev |
> | | refcnt |
> indir_dev[tun0]| ------ | cached block | ---- [ TC block ]
> | | callbacks | .
> | -------------- \__ [cb, cb_priv, cb_ident]
> | [cb, cb_priv, cb_ident]
> | --------------
> | | netdev |
> | | refcnt |
> indir_dev[tun1]| ------ | cached block | ---- [ TC block ]
> | | callbacks |.
> ----------------- -------------- \__ [cb, cb_priv, cb_ident]
> [cb, cb_priv, cb_ident]
>
>
> In the example above we have two tunnels tun0 and tun1, each one has a
> indr_dev structure allocated, and for each one of them two drivers
> registered for callbacks (hence the callbacks list has two entries).
>
> We used to cache the TC block in the indr_dev structure, but now that
> there are multiple subsytems using the indr_dev we either have to have
> a list of cached blocks (with entries for each subsystem) or just always
> iterate over the subsystems :(
>
> After all the same device may have both a TC block and a NFT block.
>
> I think always iterating would be easier:
>
> The indr_dev struct would no longer have the block pointer, instead
> when new driver registers for the callback instead of:
>
> if (indr_dev->ing_cmd_cb)
> indr_dev->ing_cmd_cb(indr_dev->dev, indr_dev->flow_block,
> indr_block_cb->cb, indr_block_cb->cb_priv,
> FLOW_BLOCK_BIND);
>
> We'd have something like the loop in flow_get_default_block():
>
> for each (subsystem)
> subsystem->handle_new_indir_cb(indr_dev, cb);
>
> And then per-subsystem logic would actually call the cb. Or:
>
> for each (subsystem)
> block = get_default_block(indir_dev)
> indr_dev->ing_cmd_cb(...)
nft dev chian is also based on register_netdevice_notifier, So for unregister case,
the basechian(block) of nft maybe delete before the __tc_indr_block_cb_unregister. is right?
So maybe we can cache the block as a list of all the subsystem in indr_dev ?
> I hope this makes sense.
>
>
> Also please double check nft unload logic has an RCU synchronization
> point, I'm not 100% confident rcu_barrier() implies synchronize_rcu().
> Perhaps someone more knowledgeable can chime in :)
>
^ permalink raw reply
* [PATCH] dpaa_eth: Use refcount_t for refcount
From: Chuhong Yuan @ 2019-08-02 10:29 UTC (permalink / raw)
Cc: Madalin Bucur, David S . Miller, netdev, linux-kernel,
Chuhong Yuan
refcount_t is better for reference counters since its
implementation can prevent overflows.
So convert atomic_t ref counters to refcount_t.
Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
---
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 6 +++---
drivers/net/ethernet/freescale/dpaa/dpaa_eth.h | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
index f38c3fa7d705..2df6e745cb3f 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
@@ -485,7 +485,7 @@ static struct dpaa_bp *dpaa_bpid2pool(int bpid)
static bool dpaa_bpid2pool_use(int bpid)
{
if (dpaa_bpid2pool(bpid)) {
- atomic_inc(&dpaa_bp_array[bpid]->refs);
+ refcount_inc(&dpaa_bp_array[bpid]->refs);
return true;
}
@@ -496,7 +496,7 @@ static bool dpaa_bpid2pool_use(int bpid)
static void dpaa_bpid2pool_map(int bpid, struct dpaa_bp *dpaa_bp)
{
dpaa_bp_array[bpid] = dpaa_bp;
- atomic_set(&dpaa_bp->refs, 1);
+ refcount_set(&dpaa_bp->refs, 1);
}
static int dpaa_bp_alloc_pool(struct dpaa_bp *dpaa_bp)
@@ -584,7 +584,7 @@ static void dpaa_bp_free(struct dpaa_bp *dpaa_bp)
if (!bp)
return;
- if (!atomic_dec_and_test(&bp->refs))
+ if (!refcount_dec_and_test(&bp->refs))
return;
if (bp->free_buf_cb)
diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h
index af320f83c742..acc3fcdf730a 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h
@@ -99,7 +99,7 @@ struct dpaa_bp {
int (*seed_cb)(struct dpaa_bp *);
/* bpool can be emptied before freeing by this cb */
void (*free_buf_cb)(const struct dpaa_bp *, struct bm_buffer *);
- atomic_t refs;
+ refcount_t refs;
};
struct dpaa_rx_errors {
--
2.20.1
^ permalink raw reply related
* [PATCH][net-next] net/mlx5: remove self-assignment on esw->dev
From: Colin King @ 2019-08-02 10:22 UTC (permalink / raw)
To: Saeed Mahameed, Leon Romanovsky, David S . Miller, netdev,
linux-rdma
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
There is a self assignment of esw->dev to itself, clean this up by
removing it.
Addresses-Coverity: ("Self assignment")
Fixes: 6cedde451399 ("net/mlx5: E-Switch, Verify support QoS element type")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/ethernet/mellanox/mlx5/core/eswitch.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
index f4ace5f8e884..de0894b695e3 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
@@ -1413,7 +1413,7 @@ static int esw_vport_egress_config(struct mlx5_eswitch *esw,
static bool element_type_supported(struct mlx5_eswitch *esw, int type)
{
- struct mlx5_core_dev *dev = esw->dev = esw->dev;
+ struct mlx5_core_dev *dev = esw->dev;
switch (type) {
case SCHEDULING_CONTEXT_ELEMENT_TYPE_TSAR:
--
2.20.1
^ permalink raw reply related
* Re: [PATCH] b43legacy: Remove pointless cond_resched() wrapper
From: Kalle Valo @ 2019-08-02 10:08 UTC (permalink / raw)
To: Thomas Gleixner; +Cc: netdev, b43-dev, Larry Finger, linux-wireless
In-Reply-To: <alpine.DEB.2.21.1907262157500.1791@nanos.tec.linutronix.de>
+ linux-wireless
Thomas Gleixner <tglx@linutronix.de> writes:
> cond_resched() can be used unconditionally. If CONFIG_PREEMPT is set, it
> becomes a NOP scheduler wise.
>
> Also the B43_BUG_ON() in that wrapper is a homebrewn variant of
> __might_sleep() which is part of cond_resched() already.
>
> Remove the wrapper and invoke cond_resched() directly.
>
> Found while looking for CONFIG_PREEMPT dependent code treewide.
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: netdev@vger.kernel.org
> Cc: b43-dev@lists.infradead.org
> Cc: Kalle Valo <kvalo@codeaurora.org>
> Cc: Larry Finger <Larry.Finger@lwfinger.net>
I use patchwork and this doesn't show there as our patchwork follows
only linux-wireless linux. Can you resend and Cc also
linux-wireless@vger.kernel.org, please?
--
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH net] ibmveth: use net_err_ratelimited when set_multicast_list
From: Hangbin Liu @ 2019-08-02 9:53 UTC (permalink / raw)
To: David Miller; +Cc: netdev, tlfalcon
In-Reply-To: <20190801.125114.1466241781699884892.davem@davemloft.net>
On Thu, Aug 01, 2019 at 12:51:14PM -0400, David Miller wrote:
> From: Hangbin Liu <liuhangbin@gmail.com>
> Date: Thu, 1 Aug 2019 17:03:47 +0800
>
> > When setting lots of multicast list on ibmveth, e.g. add 3000 membership on a
> > multicast group, the following error message flushes our log file
> >
> > 8507 [ 901.478251] ibmveth 30000003 env3: h_multicast_ctrl rc=4 when adding an entry to the filter table
> > ...
> > 1718386 [ 5636.808658] ibmveth 30000003 env3: h_multicast_ctrl rc=4 when adding an entry to the filter table
> >
> > We got 1.5 million lines of messages in 1.3h. Let's replace netdev_err() by
> > net_err_ratelimited() to avoid this issue. I don't use netdev_err_once() in
> > case h_multicast_ctrl() return different lpar_rc types.
> >
> > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
>
> Let's work on fixing what causes this problem, or adding a retry
> mechanism, rather than making the message less painful.
Yes, the multicast issue should also be fixed. It looks more like a
driver issue as I haven't seen it on other drivers.
I think these should be two different fixes.
Thanks
Hangbin
>
> What is worse is that these failures are not in any way communicated
> back up the callchain to show that the operation didn't complete
> sucessfully.
>
> This is a real mess in behavior and error handling, don't make it
> worse please.
^ permalink raw reply
* Re: [PATCH V2 7/9] vhost: do not use RCU to synchronize MMU notifier with worker
From: Jason Wang @ 2019-08-02 9:40 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: mst, kvm, virtualization, netdev, linux-kernel, linux-mm
In-Reply-To: <20190801141512.GB23899@ziepe.ca>
On 2019/8/1 下午10:15, Jason Gunthorpe wrote:
> On Thu, Aug 01, 2019 at 01:02:18PM +0800, Jason Wang wrote:
>> On 2019/8/1 上午3:30, Jason Gunthorpe wrote:
>>> On Wed, Jul 31, 2019 at 09:28:20PM +0800, Jason Wang wrote:
>>>> On 2019/7/31 下午8:39, Jason Gunthorpe wrote:
>>>>> On Wed, Jul 31, 2019 at 04:46:53AM -0400, Jason Wang wrote:
>>>>>> We used to use RCU to synchronize MMU notifier with worker. This leads
>>>>>> calling synchronize_rcu() in invalidate_range_start(). But on a busy
>>>>>> system, there would be many factors that may slow down the
>>>>>> synchronize_rcu() which makes it unsuitable to be called in MMU
>>>>>> notifier.
>>>>>>
>>>>>> A solution is SRCU but its overhead is obvious with the expensive full
>>>>>> memory barrier. Another choice is to use seqlock, but it doesn't
>>>>>> provide a synchronization method between readers and writers. The last
>>>>>> choice is to use vq mutex, but it need to deal with the worst case
>>>>>> that MMU notifier must be blocked and wait for the finish of swap in.
>>>>>>
>>>>>> So this patch switches use a counter to track whether or not the map
>>>>>> was used. The counter was increased when vq try to start or finish
>>>>>> uses the map. This means, when it was even, we're sure there's no
>>>>>> readers and MMU notifier is synchronized. When it was odd, it means
>>>>>> there's a reader we need to wait it to be even again then we are
>>>>>> synchronized.
>>>>> You just described a seqlock.
>>>> Kind of, see my explanation below.
>>>>
>>>>
>>>>> We've been talking about providing this as some core service from mmu
>>>>> notifiers because nearly every use of this API needs it.
>>>> That would be very helpful.
>>>>
>>>>
>>>>> IMHO this gets the whole thing backwards, the common pattern is to
>>>>> protect the 'shadow pte' data with a seqlock (usually open coded),
>>>>> such that the mmu notififer side has the write side of that lock and
>>>>> the read side is consumed by the thread accessing or updating the SPTE.
>>>> Yes, I've considered something like that. But the problem is, mmu notifier
>>>> (writer) need to wait for the vhost worker to finish the read before it can
>>>> do things like setting dirty pages and unmapping page. It looks to me
>>>> seqlock doesn't provide things like this.
>>> The seqlock is usually used to prevent a 2nd thread from accessing the
>>> VA while it is being changed by the mm. ie you use something seqlocky
>>> instead of the ugly mmu_notifier_unregister/register cycle.
>>
>> Yes, so we have two mappings:
>>
>> [1] vring address to VA
>> [2] VA to PA
>>
>> And have several readers and writers
>>
>> 1) set_vring_num_addr(): writer of both [1] and [2]
>> 2) MMU notifier: reader of [1] writer of [2]
>> 3) GUP: reader of [1] writer of [2]
>> 4) memory accessors: reader of [1] and [2]
>>
>> Fortunately, 1) 3) and 4) have already synchronized through vq->mutex. We
>> only need to deal with synchronization between 2) and each of the reset:
>> Sync between 1) and 2): For mapping [1], I do
>> mmu_notifier_unregister/register. This help to avoid holding any lock to do
>> overlap check.
> I suspect you could have done this with a RCU technique instead of
> register/unregister.
Probably. But the issue to be addressed by this patch is the
synchronization between MMU notifier and vhost worker.
>
>> Sync between 2) and 4): For mapping [1], both are readers, no need any
>> synchronization. For mapping [2], synchronize through RCU (or something
>> simliar to seqlock).
> You can't really use a seqlock, seqlocks are collision-retry locks,
> and the semantic here is that invalidate_range_start *MUST* not
> continue until thread doing #4 above is guarenteed no longer touching
> the memory.
Yes, that's the tricky part. For hardware like CPU, kicking through IPI
is sufficient for synchronization. But for vhost kthread, it requires a
low overhead synchronization.
>
> This must be a proper barrier, like a spinlock, mutex, or
> synchronize_rcu.
I start with synchronize_rcu() but both you and Michael raise some
concern. Then I try spinlock and mutex:
1) spinlock: add lots of overhead on datapath, this leads 0 performance
improvement.
2) SRCU: full memory barrier requires on srcu_read_lock(), which still
leads little performance improvement
3) mutex: a possible issue is need to wait for the page to be swapped in
(is this unacceptable ?), another issue is that we need hold vq lock
during range overlap check.
4) using vhost_flush_work() instead of synchronize_rcu(): still need to
wait for swap. But can do overlap checking without the lock
>
> And, again, you can't re-invent a spinlock with open coding and get
> something better.
So the question is if waiting for swap is considered to be unsuitable
for MMU notifiers. If not, it would simplify codes. If not, we still
need to figure out a possible solution.
Btw, I come up another idea, that is to disable preemption when vhost
thread need to access the memory. Then register preempt notifier and if
vhost thread is preempted, we're sure no one will access the memory and
can do the cleanup.
Thanks
>
> Jason
^ permalink raw reply
* Re: [PATCH 06/34] drm/i915: convert put_page() to put_user_page*()
From: Joonas Lahtinen @ 2019-08-02 9:19 UTC (permalink / raw)
To: Andrew Morton, john.hubbard
Cc: Christoph Hellwig, Dan Williams, Dave Chinner, Dave Hansen,
Ira Weiny, Jan Kara, Jason Gunthorpe, Jérôme Glisse,
LKML, amd-gfx, ceph-devel, devel, devel, dri-devel, intel-gfx,
kvm, linux-arm-kernel, linux-block, linux-crypto, linux-fbdev,
linux-fsdevel, linux-media, linux-mm, linux-nfs, linux-rdma,
linux-rpi-kernel, linux-xfs, netdev, rds-devel, sparclinux, x86,
xen-devel, John Hubbard, Jani Nikula, Rodrigo Vivi, David Airlie
In-Reply-To: <20190802022005.5117-7-jhubbard@nvidia.com>
Quoting john.hubbard@gmail.com (2019-08-02 05:19:37)
> From: John Hubbard <jhubbard@nvidia.com>
>
> For pages that were retained via get_user_pages*(), release those pages
> via the new put_user_page*() routines, instead of via put_page() or
> release_pages().
>
> This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
> ("mm: introduce put_user_page*(), placeholder versions").
>
> Note that this effectively changes the code's behavior in
> i915_gem_userptr_put_pages(): it now calls set_page_dirty_lock(),
> instead of set_page_dirty(). This is probably more accurate.
We've already fixed this in drm-tip where the current code uses
set_page_dirty_lock().
This would conflict with our tree. Rodrigo is handling
drm-intel-next for 5.4, so you guys want to coordinate how
to merge.
Regards, Joonas
^ permalink raw reply
* Re: [PATCH 00/34] put_user_pages(): miscellaneous call sites
From: Michal Hocko @ 2019-08-02 9:12 UTC (permalink / raw)
To: john.hubbard
Cc: Andrew Morton, Christoph Hellwig, Dan Williams, Dave Chinner,
Dave Hansen, Ira Weiny, Jan Kara, Jason Gunthorpe,
Jérôme Glisse, LKML, amd-gfx, ceph-devel, devel, devel,
dri-devel, intel-gfx, kvm, linux-arm-kernel, linux-block,
linux-crypto, linux-fbdev, linux-fsdevel, linux-media, linux-mm,
linux-nfs, linux-rdma, linux-rpi-kernel, linux-xfs, netdev,
rds-devel, sparclinux, x86, xen-devel, John Hubbard
In-Reply-To: <20190802022005.5117-1-jhubbard@nvidia.com>
On Thu 01-08-19 19:19:31, john.hubbard@gmail.com wrote:
[...]
> 2) Convert all of the call sites for get_user_pages*(), to
> invoke put_user_page*(), instead of put_page(). This involves dozens of
> call sites, and will take some time.
How do we make sure this is the case and it will remain the case in the
future? There must be some automagic to enforce/check that. It is simply
not manageable to do it every now and then because then 3) will simply
be never safe.
Have you considered coccinele or some other scripted way to do the
transition? I have no idea how to deal with future changes that would
break the balance though.
--
Michal Hocko
SUSE Labs
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox