From: Jon Paul Maloy Date: Thu, 13 Feb 2014 22:29:18 +0000 (-0500) Subject: tipc: add node_lock protection to link lookup function X-Git-Url: http://git.cdn.openwrt.org/?a=commitdiff_plain;h=e099e86c9e24fe9aff36773600543eb31d8954d;p=openwrt%2Fstaging%2Fblogic.git tipc: add node_lock protection to link lookup function In an earlier commit, ("tipc: remove links list from bearer struct") we described three issues that need to be pre-emptively resolved before we can remove tipc_net_lock. Here we resolve issue a) described in that commit: "a) In access method #2, we access the link before taking the protecting node_lock. This will not work once net_lock is gone, so we will have to change the access order. We will deal with this in a later commit in this series." Here, we change that access order, by ensuring that the function link_find_link() returns only a safe reference for finding the link, i.e., a node pointer and an index into its 'links' array, not the link pointer itself. We also change all callers of this function to first take the node lock before they can check if there still is a valid link pointer at the returned index. Since the function now returns a node pointer rather than a link pointer, we rename it to the more appropriate 'tipc_link_find_owner(). Signed-off-by: Jon Maloy Reviewed-by: Ying Xue Signed-off-by: David S. Miller --- diff --git a/net/tipc/link.c b/net/tipc/link.c index 03075165665e..4fb4ae0a75ed 100644 --- a/net/tipc/link.c +++ b/net/tipc/link.c @@ -2390,35 +2390,40 @@ void tipc_link_set_queue_limits(struct tipc_link *l_ptr, u32 window) l_ptr->queue_limit[MSG_FRAGMENTER] = 4000; } -/** - * link_find_link - locate link by name - * @name: ptr to link name string - * @node: ptr to area to be filled with ptr to associated node - * +/* tipc_link_find_owner - locate owner node of link by link's name + * @name: pointer to link name string + * @bearer_id: pointer to index in 'node->links' array where the link was found. * Caller must hold 'tipc_net_lock' to ensure node and bearer are not deleted; * this also prevents link deletion. * - * Returns pointer to link (or 0 if invalid link name). + * Returns pointer to node owning the link, or 0 if no matching link is found. */ -static struct tipc_link *link_find_link(const char *name, - struct tipc_node **node) +static struct tipc_node *tipc_link_find_owner(const char *link_name, + unsigned int *bearer_id) { struct tipc_link *l_ptr; struct tipc_node *n_ptr; + struct tipc_node *tmp_n_ptr; + struct tipc_node *found_node = 0; + int i; - list_for_each_entry(n_ptr, &tipc_node_list, list) { + *bearer_id = 0; + list_for_each_entry_safe(n_ptr, tmp_n_ptr, &tipc_node_list, list) { + spin_lock(&n_ptr->lock); for (i = 0; i < MAX_BEARERS; i++) { l_ptr = n_ptr->links[i]; - if (l_ptr && !strcmp(l_ptr->name, name)) - goto found; + if (l_ptr && !strcmp(l_ptr->name, link_name)) { + *bearer_id = i; + found_node = n_ptr; + break; + } } + spin_unlock(&n_ptr->lock); + if (found_node) + break; } - l_ptr = NULL; - n_ptr = NULL; -found: - *node = n_ptr; - return l_ptr; + return found_node; } /** @@ -2460,32 +2465,33 @@ static int link_cmd_set_value(const char *name, u32 new_value, u16 cmd) struct tipc_link *l_ptr; struct tipc_bearer *b_ptr; struct tipc_media *m_ptr; + int bearer_id; int res = 0; - l_ptr = link_find_link(name, &node); - if (l_ptr) { - /* - * acquire node lock for tipc_link_send_proto_msg(). - * see "TIPC locking policy" in net.c. - */ + node = tipc_link_find_owner(name, &bearer_id); + if (node) { tipc_node_lock(node); - switch (cmd) { - case TIPC_CMD_SET_LINK_TOL: - link_set_supervision_props(l_ptr, new_value); - tipc_link_send_proto_msg(l_ptr, - STATE_MSG, 0, 0, new_value, 0, 0); - break; - case TIPC_CMD_SET_LINK_PRI: - l_ptr->priority = new_value; - tipc_link_send_proto_msg(l_ptr, - STATE_MSG, 0, 0, 0, new_value, 0); - break; - case TIPC_CMD_SET_LINK_WINDOW: - tipc_link_set_queue_limits(l_ptr, new_value); - break; - default: - res = -EINVAL; - break; + l_ptr = node->links[bearer_id]; + + if (l_ptr) { + switch (cmd) { + case TIPC_CMD_SET_LINK_TOL: + link_set_supervision_props(l_ptr, new_value); + tipc_link_send_proto_msg(l_ptr, STATE_MSG, 0, + 0, new_value, 0, 0); + break; + case TIPC_CMD_SET_LINK_PRI: + l_ptr->priority = new_value; + tipc_link_send_proto_msg(l_ptr, STATE_MSG, 0, + 0, 0, new_value, 0); + break; + case TIPC_CMD_SET_LINK_WINDOW: + tipc_link_set_queue_limits(l_ptr, new_value); + break; + default: + res = -EINVAL; + break; + } } tipc_node_unlock(node); return res; @@ -2580,6 +2586,7 @@ struct sk_buff *tipc_link_cmd_reset_stats(const void *req_tlv_area, int req_tlv_ char *link_name; struct tipc_link *l_ptr; struct tipc_node *node; + unsigned int bearer_id; if (!TLV_CHECK(req_tlv_area, req_tlv_space, TIPC_TLV_LINK_NAME)) return tipc_cfg_reply_error_string(TIPC_CFG_TLV_ERROR); @@ -2590,15 +2597,19 @@ struct sk_buff *tipc_link_cmd_reset_stats(const void *req_tlv_area, int req_tlv_ return tipc_cfg_reply_error_string("link not found"); return tipc_cfg_reply_none(); } - read_lock_bh(&tipc_net_lock); - l_ptr = link_find_link(link_name, &node); + node = tipc_link_find_owner(link_name, &bearer_id); + if (!node) { + read_unlock_bh(&tipc_net_lock); + return tipc_cfg_reply_error_string("link not found"); + } + spin_lock(&node->lock); + l_ptr = node->links[bearer_id]; if (!l_ptr) { + tipc_node_unlock(node); read_unlock_bh(&tipc_net_lock); return tipc_cfg_reply_error_string("link not found"); } - - tipc_node_lock(node); link_reset_statistics(l_ptr); tipc_node_unlock(node); read_unlock_bh(&tipc_net_lock); @@ -2628,18 +2639,27 @@ static int tipc_link_stats(const char *name, char *buf, const u32 buf_size) struct tipc_node *node; char *status; u32 profile_total = 0; + unsigned int bearer_id; int ret; if (!strcmp(name, tipc_bclink_name)) return tipc_bclink_stats(buf, buf_size); read_lock_bh(&tipc_net_lock); - l = link_find_link(name, &node); - if (!l) { + node = tipc_link_find_owner(name, &bearer_id); + if (!node) { read_unlock_bh(&tipc_net_lock); return 0; } tipc_node_lock(node); + + l = node->links[bearer_id]; + if (!l) { + tipc_node_unlock(node); + read_unlock_bh(&tipc_net_lock); + return 0; + } + s = &l->stats; if (tipc_link_is_active(l))