diff mbox series

[net,v2,1/1] tipc: fix kernel panic when enabling bearer

Message ID 20220304032518.9305-1-tung.q.nguyen@dektech.com.au (mailing list archive)
State Accepted
Commit be4977b847f5d5cedb64d50eaaf2218c3a55a3a3
Delegated to: Netdev Maintainers
Headers show
Series [net,v2,1/1] tipc: fix kernel panic when enabling bearer | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net
netdev/fixes_present success Fixes tag present in non-next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers fail 1 blamed authors not CCed: ying.xue@windriver.com; 1 maintainers not CCed: ying.xue@windriver.com
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 23 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Tung Quang Nguyen March 4, 2022, 3:25 a.m. UTC
When enabling a bearer on a node, a kernel panic is observed:

[    4.498085] RIP: 0010:tipc_mon_prep+0x4e/0x130 [tipc]
...
[    4.520030] Call Trace:
[    4.520689]  <IRQ>
[    4.521236]  tipc_link_build_proto_msg+0x375/0x750 [tipc]
[    4.522654]  tipc_link_build_state_msg+0x48/0xc0 [tipc]
[    4.524034]  __tipc_node_link_up+0xd7/0x290 [tipc]
[    4.525292]  tipc_rcv+0x5da/0x730 [tipc]
[    4.526346]  ? __netif_receive_skb_core+0xb7/0xfc0
[    4.527601]  tipc_l2_rcv_msg+0x5e/0x90 [tipc]
[    4.528737]  __netif_receive_skb_list_core+0x20b/0x260
[    4.530068]  netif_receive_skb_list_internal+0x1bf/0x2e0
[    4.531450]  ? dev_gro_receive+0x4c2/0x680
[    4.532512]  napi_complete_done+0x6f/0x180
[    4.533570]  virtnet_poll+0x29c/0x42e [virtio_net]
...

The node in question is receiving activate messages in another
thread after changing bearer status to allow message sending/
receiving in current thread:

         thread 1           |              thread 2
         --------           |              --------
                            |
tipc_enable_bearer()        |
  test_and_set_bit_lock()   |
    tipc_bearer_xmit_skb()  |
                            | tipc_l2_rcv_msg()
                            |   tipc_rcv()
                            |     __tipc_node_link_up()
                            |       tipc_link_build_state_msg()
                            |         tipc_link_build_proto_msg()
                            |           tipc_mon_prep()
                            |           {
                            |             ...
                            |             // null-pointer dereference
                            |             u16 gen = mon->dom_gen;
                            |             ...
                            |           }
  // Not being executed yet |
  tipc_mon_create()         |
  {                         |
    ...                     |
    // allocate             |
    mon = kzalloc();        |
    ...                     |
  }                         |

Monitoring pointer in thread 2 is dereferenced before monitoring data
is allocated in thread 1. This causes kernel panic.

This commit fixes it by allocating the monitoring data before enabling
the bearer to receive messages.

Fixes: 35c55c9877f8 ("tipc: add neighbor monitoring framework")
Reported-by: Shuang Li <shuali@redhat.com>
Acked-by: Jon Maloy <jmaloy@redhat.com>
Signed-off-by: Tung Nguyen <tung.q.nguyen@dektech.com.au>
---
 net/tipc/bearer.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

Comments

patchwork-bot+netdevbpf@kernel.org March 4, 2022, 1:20 p.m. UTC | #1
Hello:

This patch was applied to netdev/net.git (master)
by David S. Miller <davem@davemloft.net>:

On Fri,  4 Mar 2022 03:25:18 +0000 you wrote:
> When enabling a bearer on a node, a kernel panic is observed:
> 
> [    4.498085] RIP: 0010:tipc_mon_prep+0x4e/0x130 [tipc]
> ...
> [    4.520030] Call Trace:
> [    4.520689]  <IRQ>
> [    4.521236]  tipc_link_build_proto_msg+0x375/0x750 [tipc]
> [    4.522654]  tipc_link_build_state_msg+0x48/0xc0 [tipc]
> [    4.524034]  __tipc_node_link_up+0xd7/0x290 [tipc]
> [    4.525292]  tipc_rcv+0x5da/0x730 [tipc]
> [    4.526346]  ? __netif_receive_skb_core+0xb7/0xfc0
> [    4.527601]  tipc_l2_rcv_msg+0x5e/0x90 [tipc]
> [    4.528737]  __netif_receive_skb_list_core+0x20b/0x260
> [    4.530068]  netif_receive_skb_list_internal+0x1bf/0x2e0
> [    4.531450]  ? dev_gro_receive+0x4c2/0x680
> [    4.532512]  napi_complete_done+0x6f/0x180
> [    4.533570]  virtnet_poll+0x29c/0x42e [virtio_net]
> ...
> 
> [...]

Here is the summary with links:
  - [net,v2,1/1] tipc: fix kernel panic when enabling bearer
    https://git.kernel.org/netdev/net/c/be4977b847f5

You are awesome, thank you!
diff mbox series

Patch

diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
index 473a790f5894..a2f9c9640716 100644
--- a/net/tipc/bearer.c
+++ b/net/tipc/bearer.c
@@ -352,16 +352,18 @@  static int tipc_enable_bearer(struct net *net, const char *name,
 		goto rejected;
 	}
 
-	test_and_set_bit_lock(0, &b->up);
-	rcu_assign_pointer(tn->bearer_list[bearer_id], b);
-	if (skb)
-		tipc_bearer_xmit_skb(net, bearer_id, skb, &b->bcast_addr);
-
+	/* Create monitoring data before accepting activate messages */
 	if (tipc_mon_create(net, bearer_id)) {
 		bearer_disable(net, b);
+		kfree_skb(skb);
 		return -ENOMEM;
 	}
 
+	test_and_set_bit_lock(0, &b->up);
+	rcu_assign_pointer(tn->bearer_list[bearer_id], b);
+	if (skb)
+		tipc_bearer_xmit_skb(net, bearer_id, skb, &b->bcast_addr);
+
 	pr_info("Enabled bearer <%s>, priority %u\n", name, prio);
 
 	return res;