| From d5f9023fa61ee8b94f37a93f08e94b136cf1e463 Mon Sep 17 00:00:00 2001 |
| From: Thadeu Lima de Souza Cascardo <cascardo@canonical.com> |
| Date: Sat, 19 Jun 2021 13:18:13 -0300 |
| Subject: can: bcm: delay release of struct bcm_op after synchronize_rcu() |
| |
| 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> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| net/can/bcm.c | 7 ++++++- |
| 1 file changed, 6 insertions(+), 1 deletion(-) |
| |
| --- a/net/can/bcm.c |
| +++ b/net/can/bcm.c |
| @@ -785,6 +785,7 @@ static int bcm_delete_rx_op(struct list_ |
| bcm_rx_handler, op); |
| |
| list_del(&op->list); |
| + synchronize_rcu(); |
| bcm_remove_op(op); |
| return 1; /* done */ |
| } |
| @@ -1533,9 +1534,13 @@ static int bcm_release(struct socket *so |
| 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); |
| + |
| #if IS_ENABLED(CONFIG_PROC_FS) |
| /* remove procfs entry */ |
| if (net->can.bcmproc_dir && bo->bcm_proc_read) |