RE: [PATCH net v6] tipc: serialize udp bearer replicast list updates

[Tung Quang Nguyen]

>Subject: [PATCH net v6] tipc: serialize udp bearer replicast list updates
>
>tipc_udp_rcast_add() and cleanup_bearer() both update ub->rcast.list with
>list_add_rcu() / list_del_rcu(), but nothing serializes them. The add runs from
>the encap receive softirq (via tipc_udp_rcast_disc()) without rtnl_lock(), so it
>can race the cleanup delete and corrupt the list:
>
>  list_del corruption. prev->next should be ffff8880298d7ab8,
>    but was ffff88802449ad38. (prev=ffff888027e3ec98)
>  kernel BUG at lib/list_debug.c:62!
>  RIP: __list_del_entry_valid_or_report+0x17a/0x200
>  Workqueue: events cleanup_bearer
>  Call Trace:
>   cleanup_bearer (net/tipc/udp_media.c:811)
>   process_one_work (kernel/workqueue.c:3302)
>   worker_thread (kernel/workqueue.c:3466)
>
>The bearer can be enabled from an unprivileged user namespace, as the
>TIPCv2 generic-netlink ops carry no GENL_ADMIN_PERM.
>
>Add a spinlock to struct udp_bearer and take it around the list_add_rcu() in
>tipc_udp_rcast_add() and the list_del_rcu() loop in cleanup_bearer() so the
>two writers can no longer corrupt the list.
>
>Reject a duplicate peer under the same lock before allocating, and remove
>tipc_udp_is_known_peer(). The old lockless pre-check in
>tipc_udp_rcast_disc() was racy: two softirqs discovering the same peer could
>both find it absent and add it twice.
>
>cleanup_bearer() runs from a workqueue after tipc_udp_disable() clears the
>bearer's up bit, so an encap softirq can still reach tipc_udp_rcast_add() and
>add a peer after cleanup_bearer() has already emptied the list, leaking that
>entry when the bearer is freed. Mark the bearer disabled under rcast_lock
>once the list is emptied and refuse further additions.
>
Reviewed-by: Tung Nguyen <tung.quang.nguyen@xxxxxxxx>