On 01/03/2018 11:48 PM, David Miller wrote:
> As soon as you drop the RTNL lock, the media or bearer entry can be
> removed from the tables.
>
Thanks for the review. Yes, you are right. But even if we temporarily
release RTNL lock, it's still safe for us because when we set
media/bearer properties in __tipc_nl_compat_doit(), tipc_nl_media_set()
and tipc_nl_bearer_set() will probe media or bearer again within RTNL
lock protection.
> This invalidates what you do next, whether it's
> tipc_nl_compat_media_set(), tipc_nl_compat_bearer_set(), etc.
In fact tipc_nl_compat_media_set() and tipc_nl_compat_bearer_set() don't
really change media or bearer's properties, instead they only format the
contents pointed by their "msg" parameter.
>
> Therefore, you have to lock down the tipc configuration state around
> this entire operation, from media/bearer probe to the building of the
> netlink message(s).
>
Sorry, we cannot hold RTNL lock in the entire operation path because
TIPC now supports two different sets of netlink APIs:
One set of API's execution path:
genl_family_rcv_msg()
tipc_nl_media_set()
rtnl_lock()
tipc_media_find()
//set media properties
genl_family_rcv_msg()
tipc_nl_bearer_set()
rtnl_lock()
tipc_bearer_find()
//set bearer properties
Another set of API's execution path:
genl_family_rcv_msg()
tipc_nl_compat_recv()
tipc_nl_compat_handle net()
__tipc_nl_compat_doit net()
tipc_nl_compat_link_set()
tipc_nl_media_set()
genl_family_rcv_msg()
tipc_nl_compat_recv()
tipc_nl_compat_handle net()
__tipc_nl_compat_doit net()
tipc_nl_compat_link_set()
tipc_nl_bearer_set()
As we see in above call chains, tipc_nl_media_set() and
tipc_nl_bearer_set() are shared by the two sets of netlink APIs. If we
hold RTNL lock from tipc_nl_compat_recv(), it means we cannot directly
call tipc_nl_media_set() or tipc_nl_bearer_set() in
__tipc_nl_compat_doit net().
> Either this entire code path must execute with the bearer/media entry
> present, or without. If you drop the RTNL mutex in the middle, this
> invariant is not held.
