Skip to content

Commit

Permalink
UBUNTU: SAUCE: can: bcm: delay release of struct bcm_op after synchro…
Browse files Browse the repository at this point in the history
…nize_rcu

BugLink: https://bugs.launchpad.net/bugs/1931855 (UAF on CAN BCM bcm_rx_handler (LP: #1931855))

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.

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: Benjamin M Romer <benjamin.romer@canonical.com>
Acked-by: Ian May <ian.may@canonical.com>
  • Loading branch information
Thadeu Lima de Souza Cascardo committed Jun 16, 2021
1 parent 50367e7 commit a9b1054
Showing 1 changed file with 6 additions and 0 deletions.
6 changes: 6 additions & 0 deletions net/can/bcm.c
Original file line number Diff line number Diff line change
Expand Up @@ -780,6 +780,7 @@ static int bcm_delete_rx_op(struct list_head *ops, struct bcm_msg_head *mh,
bcm_rx_handler, op);

list_del(&op->list);
synchronize_rcu();
bcm_remove_op(op);
return 1; /* done */
}
Expand Down Expand Up @@ -1505,6 +1506,11 @@ static int bcm_release(struct socket *sock)
REGMASK(op->can_id),
bcm_rx_handler, op);

}

synchronize_rcu();

list_for_each_entry_safe(op, next, &bo->rx_ops, list) {
bcm_remove_op(op);
}

Expand Down

0 comments on commit a9b1054

Please sign in to comment.