From: Oliver Hartkopp socketcan@hartkopp.net
commit fb8696ab14adadb2e3f6c17c18ed26b3ecd96691 upstream.
can_can_gw_rcv() is called under RCU protection, so after calling can_rx_unregister(), we have to call synchronize_rcu in order to wait for any RCU read-side critical sections to finish before removing the kmem_cache entry with the referenced gw job entry.
Link: https://lore.kernel.org/r/20210618173645.2238-1-socketcan@hartkopp.net Fixes: c1aabdf379bc ("can-gw: add netlink based CAN routing") Cc: linux-stable stable@vger.kernel.org Signed-off-by: Oliver Hartkopp socketcan@hartkopp.net Signed-off-by: Marc Kleine-Budde mkl@pengutronix.de --- net/can/gw.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/net/can/gw.c b/net/can/gw.c index 81650affa3fa..1867000f8a65 100644 --- a/net/can/gw.c +++ b/net/can/gw.c @@ -495,10 +495,11 @@ static int cgw_notifier(struct notifier_block *nb, hlist_for_each_entry_safe(gwj, nx, &cgw_list, list) {
if (gwj->src.dev == dev || gwj->dst.dev == dev) { hlist_del(&gwj->list); cgw_unregister_filter(gwj); + synchronize_rcu(); kmem_cache_free(cgw_cache, gwj); } } }
@@ -939,10 +940,11 @@ static void cgw_remove_all_jobs(void) ASSERT_RTNL();
hlist_for_each_entry_safe(gwj, nx, &cgw_list, list) { hlist_del(&gwj->list); cgw_unregister_filter(gwj); + synchronize_rcu(); kmem_cache_free(cgw_cache, gwj); } }
static int cgw_remove_job(struct sk_buff *skb, struct nlmsghdr *nlh) @@ -1006,10 +1008,11 @@ static int cgw_remove_job(struct sk_buff *skb, struct nlmsghdr *nlh) if (memcmp(&gwj->ccgw, &ccgw, sizeof(ccgw))) continue;
hlist_del(&gwj->list); cgw_unregister_filter(gwj); + synchronize_rcu(); kmem_cache_free(cgw_cache, gwj); err = 0; break; }
From: Thadeu Lima de Souza Cascardo cascardo@canonical.com
From: Thadeu Lima de Souza Cascardo cascardo@canonical.com
commit d5f9023fa61ee8b94f37a93f08e94b136cf1e463 upstream.
can_rx_register() callbacks may be called concurrently to the call to can_rx_unregister(). The callbacks and callback data, though, are protected by RCU and the struct sock reference count.
So the callback data is really attached to the life of sk, meaning that it should be released on sk_destruct. However, bcm_remove_op() calls tasklet_kill(), and RCU callbacks may be called under RCU softirq, so that cannot be used on kernels before the introduction of HRTIMER_MODE_SOFT.
However, bcm_rx_handler() is called under RCU protection, so after calling can_rx_unregister(), we may call synchronize_rcu() in order to wait for any RCU read-side critical sections to finish. That is, bcm_rx_handler() won't be called anymore for those ops. So, we only free them, after we do that synchronize_rcu().
Fixes: ffd980f976e7 ("[CAN]: Add broadcast manager (bcm) protocol") Link: https://lore.kernel.org/r/20210619161813.2098382-1-cascardo@canonical.com Cc: linux-stable stable@vger.kernel.org Reported-by: syzbot+0f7e7e5e2f4f40fa89c0@syzkaller.appspotmail.com Reported-by: Norbert Slusarek nslusarek@gmx.net Signed-off-by: Thadeu Lima de Souza Cascardo cascardo@canonical.com Acked-by: Oliver Hartkopp socketcan@hartkopp.net Signed-off-by: Marc Kleine-Budde mkl@pengutronix.de --- net/can/bcm.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/net/can/bcm.c b/net/can/bcm.c index c6fee58baac4..3e131dc5f0e5 100644 --- a/net/can/bcm.c +++ b/net/can/bcm.c @@ -811,10 +811,11 @@ static int bcm_delete_rx_op(struct list_head *ops, canid_t can_id, int ifindex) can_rx_unregister(NULL, op->can_id, REGMASK(op->can_id), bcm_rx_handler, op);
list_del(&op->list); + synchronize_rcu(); bcm_remove_op(op); return 1; /* done */ } }
@@ -1536,13 +1537,17 @@ static int bcm_release(struct socket *sock) } else can_rx_unregister(NULL, op->can_id, REGMASK(op->can_id), bcm_rx_handler, op);
- bcm_remove_op(op); }
+ synchronize_rcu(); + + list_for_each_entry_safe(op, next, &bo->rx_ops, list) + bcm_remove_op(op); + /* remove procfs entry */ if (proc_dir && bo->bcm_proc_read) remove_proc_entry(bo->procname, proc_dir);
/* remove device reference */
On Mon, Jul 12, 2021 at 05:30:35PM +0200, Oliver Hartkopp wrote:
From: Oliver Hartkopp socketcan@hartkopp.net
commit fb8696ab14adadb2e3f6c17c18ed26b3ecd96691 upstream.
can_can_gw_rcv() is called under RCU protection, so after calling can_rx_unregister(), we have to call synchronize_rcu in order to wait for any RCU read-side critical sections to finish before removing the kmem_cache entry with the referenced gw job entry.
Link: https://lore.kernel.org/r/20210618173645.2238-1-socketcan@hartkopp.net Fixes: c1aabdf379bc ("can-gw: add netlink based CAN routing") Cc: linux-stable stable@vger.kernel.org Signed-off-by: Oliver Hartkopp socketcan@hartkopp.net Signed-off-by: Marc Kleine-Budde mkl@pengutronix.de
net/can/gw.c | 3 +++ 1 file changed, 3 insertions(+)
Both now queued up, thanks.
greg k-h
linux-stable-mirror@lists.linaro.org
-
Greg KH -
Oliver Hartkopp