[PATCH stable 4.4 4.9] can: gw: synchronize rcu operations before removing gw job entry

[PATCH stable 4.4 4.9] can: gw: synchronize rcu operations before removing gw job entry

[Oliver Hartkopp]

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;
 	}
 
-- 
2.30.2

[PATCH stable 4.4 4.9] can: bcm: delay release of struct bcm_op after synchronize_rcu()

[Oliver Hartkopp]

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 */
-- 
2.30.2

Re: [PATCH stable 4.4 4.9] can: gw: synchronize rcu operations before removing gw job entry

[Greg KH]

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