Commit cb8092d7 authored by Xin Long's avatar Xin Long Committed by David S. Miller

tipc: move bc link creation back to tipc_node_create

Shuang Li reported a NULL pointer dereference crash:

  [] BUG: kernel NULL pointer dereference, address: 0000000000000068
  [] RIP: 0010:tipc_link_is_up+0x5/0x10 [tipc]
  [] Call Trace:
  []  <IRQ>
  []  tipc_bcast_rcv+0xa2/0x190 [tipc]
  []  tipc_node_bc_rcv+0x8b/0x200 [tipc]
  []  tipc_rcv+0x3af/0x5b0 [tipc]
  []  tipc_udp_recv+0xc7/0x1e0 [tipc]

It was caused by the 'l' passed into tipc_bcast_rcv() is NULL. When it
creates a node in tipc_node_check_dest(), after inserting the new node
into hashtable in tipc_node_create(), it creates the bc link. However,
there is a gap between this insert and bc link creation, a bc packet
may come in and get the node from the hashtable then try to dereference
its bc link, which is NULL.

This patch is to fix it by moving the bc link creation before inserting
into the hashtable.

Note that for a preliminary node becoming "real", the bc link creation
should also be called before it's rehashed, as we don't create it for
preliminary nodes.

Fixes: 4cbf8ac2 ("tipc: enable creating a "preliminary" node")
Reported-by: default avatarShuang Li <shuali@redhat.com>
Signed-off-by: default avatarXin Long <lucien.xin@gmail.com>
Acked-by: default avatarJon Maloy <jmaloy@redhat.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 853a7614
...@@ -472,8 +472,8 @@ struct tipc_node *tipc_node_create(struct net *net, u32 addr, u8 *peer_id, ...@@ -472,8 +472,8 @@ struct tipc_node *tipc_node_create(struct net *net, u32 addr, u8 *peer_id,
bool preliminary) bool preliminary)
{ {
struct tipc_net *tn = net_generic(net, tipc_net_id); struct tipc_net *tn = net_generic(net, tipc_net_id);
struct tipc_link *l, *snd_l = tipc_bc_sndlink(net);
struct tipc_node *n, *temp_node; struct tipc_node *n, *temp_node;
struct tipc_link *l;
unsigned long intv; unsigned long intv;
int bearer_id; int bearer_id;
int i; int i;
...@@ -488,6 +488,16 @@ struct tipc_node *tipc_node_create(struct net *net, u32 addr, u8 *peer_id, ...@@ -488,6 +488,16 @@ struct tipc_node *tipc_node_create(struct net *net, u32 addr, u8 *peer_id,
goto exit; goto exit;
/* A preliminary node becomes "real" now, refresh its data */ /* A preliminary node becomes "real" now, refresh its data */
tipc_node_write_lock(n); tipc_node_write_lock(n);
if (!tipc_link_bc_create(net, tipc_own_addr(net), addr, peer_id, U16_MAX,
tipc_link_min_win(snd_l), tipc_link_max_win(snd_l),
n->capabilities, &n->bc_entry.inputq1,
&n->bc_entry.namedq, snd_l, &n->bc_entry.link)) {
pr_warn("Broadcast rcv link refresh failed, no memory\n");
tipc_node_write_unlock_fast(n);
tipc_node_put(n);
n = NULL;
goto exit;
}
n->preliminary = false; n->preliminary = false;
n->addr = addr; n->addr = addr;
hlist_del_rcu(&n->hash); hlist_del_rcu(&n->hash);
...@@ -567,7 +577,16 @@ struct tipc_node *tipc_node_create(struct net *net, u32 addr, u8 *peer_id, ...@@ -567,7 +577,16 @@ struct tipc_node *tipc_node_create(struct net *net, u32 addr, u8 *peer_id,
n->signature = INVALID_NODE_SIG; n->signature = INVALID_NODE_SIG;
n->active_links[0] = INVALID_BEARER_ID; n->active_links[0] = INVALID_BEARER_ID;
n->active_links[1] = INVALID_BEARER_ID; n->active_links[1] = INVALID_BEARER_ID;
n->bc_entry.link = NULL; if (!preliminary &&
!tipc_link_bc_create(net, tipc_own_addr(net), addr, peer_id, U16_MAX,
tipc_link_min_win(snd_l), tipc_link_max_win(snd_l),
n->capabilities, &n->bc_entry.inputq1,
&n->bc_entry.namedq, snd_l, &n->bc_entry.link)) {
pr_warn("Broadcast rcv link creation failed, no memory\n");
kfree(n);
n = NULL;
goto exit;
}
tipc_node_get(n); tipc_node_get(n);
timer_setup(&n->timer, tipc_node_timeout, 0); timer_setup(&n->timer, tipc_node_timeout, 0);
/* Start a slow timer anyway, crypto needs it */ /* Start a slow timer anyway, crypto needs it */
...@@ -1155,7 +1174,7 @@ void tipc_node_check_dest(struct net *net, u32 addr, ...@@ -1155,7 +1174,7 @@ void tipc_node_check_dest(struct net *net, u32 addr,
bool *respond, bool *dupl_addr) bool *respond, bool *dupl_addr)
{ {
struct tipc_node *n; struct tipc_node *n;
struct tipc_link *l, *snd_l; struct tipc_link *l;
struct tipc_link_entry *le; struct tipc_link_entry *le;
bool addr_match = false; bool addr_match = false;
bool sign_match = false; bool sign_match = false;
...@@ -1175,22 +1194,6 @@ void tipc_node_check_dest(struct net *net, u32 addr, ...@@ -1175,22 +1194,6 @@ void tipc_node_check_dest(struct net *net, u32 addr,
return; return;
tipc_node_write_lock(n); tipc_node_write_lock(n);
if (unlikely(!n->bc_entry.link)) {
snd_l = tipc_bc_sndlink(net);
if (!tipc_link_bc_create(net, tipc_own_addr(net),
addr, peer_id, U16_MAX,
tipc_link_min_win(snd_l),
tipc_link_max_win(snd_l),
n->capabilities,
&n->bc_entry.inputq1,
&n->bc_entry.namedq, snd_l,
&n->bc_entry.link)) {
pr_warn("Broadcast rcv link creation failed, no mem\n");
tipc_node_write_unlock_fast(n);
tipc_node_put(n);
return;
}
}
le = &n->links[b->identity]; le = &n->links[b->identity];
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment