]>
Commit | Line | Data |
---|---|---|
36e3f504 SL |
1 | From 255776ca65596b371b872f317aa5966bb007b17f Mon Sep 17 00:00:00 2001 |
2 | From: Sean Tranchetti <stranche@codeaurora.org> | |
3 | Date: Thu, 7 Feb 2019 13:33:21 -0700 | |
4 | Subject: af_key: unconditionally clone on broadcast | |
5 | ||
6 | [ Upstream commit fc2d5cfdcfe2ab76b263d91429caa22451123085 ] | |
7 | ||
8 | Attempting to avoid cloning the skb when broadcasting by inflating | |
9 | the refcount with sock_hold/sock_put while under RCU lock is dangerous | |
10 | and violates RCU principles. It leads to subtle race conditions when | |
11 | attempting to free the SKB, as we may reference sockets that have | |
12 | already been freed by the stack. | |
13 | ||
14 | Unable to handle kernel paging request at virtual address 6b6b6b6b6b6c4b | |
15 | [006b6b6b6b6b6c4b] address between user and kernel address ranges | |
16 | Internal error: Oops: 96000004 [#1] PREEMPT SMP | |
17 | task: fffffff78f65b380 task.stack: ffffff8049a88000 | |
18 | pc : sock_rfree+0x38/0x6c | |
19 | lr : skb_release_head_state+0x6c/0xcc | |
20 | Process repro (pid: 7117, stack limit = 0xffffff8049a88000) | |
21 | Call trace: | |
22 | sock_rfree+0x38/0x6c | |
23 | skb_release_head_state+0x6c/0xcc | |
24 | skb_release_all+0x1c/0x38 | |
25 | __kfree_skb+0x1c/0x30 | |
26 | kfree_skb+0xd0/0xf4 | |
27 | pfkey_broadcast+0x14c/0x18c | |
28 | pfkey_sendmsg+0x1d8/0x408 | |
29 | sock_sendmsg+0x44/0x60 | |
30 | ___sys_sendmsg+0x1d0/0x2a8 | |
31 | __sys_sendmsg+0x64/0xb4 | |
32 | SyS_sendmsg+0x34/0x4c | |
33 | el0_svc_naked+0x34/0x38 | |
34 | Kernel panic - not syncing: Fatal exception | |
35 | ||
36 | Suggested-by: Eric Dumazet <eric.dumazet@gmail.com> | |
37 | Signed-off-by: Sean Tranchetti <stranche@codeaurora.org> | |
38 | Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com> | |
39 | Signed-off-by: Sasha Levin <sashal@kernel.org> | |
40 | --- | |
41 | net/key/af_key.c | 40 +++++++++++++++------------------------- | |
42 | 1 file changed, 15 insertions(+), 25 deletions(-) | |
43 | ||
44 | diff --git a/net/key/af_key.c b/net/key/af_key.c | |
45 | index 9d61266526e7..7da629d59717 100644 | |
46 | --- a/net/key/af_key.c | |
47 | +++ b/net/key/af_key.c | |
48 | @@ -196,30 +196,22 @@ static int pfkey_release(struct socket *sock) | |
49 | return 0; | |
50 | } | |
51 | ||
52 | -static int pfkey_broadcast_one(struct sk_buff *skb, struct sk_buff **skb2, | |
53 | - gfp_t allocation, struct sock *sk) | |
54 | +static int pfkey_broadcast_one(struct sk_buff *skb, gfp_t allocation, | |
55 | + struct sock *sk) | |
56 | { | |
57 | int err = -ENOBUFS; | |
58 | ||
59 | - sock_hold(sk); | |
60 | - if (*skb2 == NULL) { | |
61 | - if (refcount_read(&skb->users) != 1) { | |
62 | - *skb2 = skb_clone(skb, allocation); | |
63 | - } else { | |
64 | - *skb2 = skb; | |
65 | - refcount_inc(&skb->users); | |
66 | - } | |
67 | - } | |
68 | - if (*skb2 != NULL) { | |
69 | - if (atomic_read(&sk->sk_rmem_alloc) <= sk->sk_rcvbuf) { | |
70 | - skb_set_owner_r(*skb2, sk); | |
71 | - skb_queue_tail(&sk->sk_receive_queue, *skb2); | |
72 | - sk->sk_data_ready(sk); | |
73 | - *skb2 = NULL; | |
74 | - err = 0; | |
75 | - } | |
76 | + if (atomic_read(&sk->sk_rmem_alloc) > sk->sk_rcvbuf) | |
77 | + return err; | |
78 | + | |
79 | + skb = skb_clone(skb, allocation); | |
80 | + | |
81 | + if (skb) { | |
82 | + skb_set_owner_r(skb, sk); | |
83 | + skb_queue_tail(&sk->sk_receive_queue, skb); | |
84 | + sk->sk_data_ready(sk); | |
85 | + err = 0; | |
86 | } | |
87 | - sock_put(sk); | |
88 | return err; | |
89 | } | |
90 | ||
91 | @@ -234,7 +226,6 @@ static int pfkey_broadcast(struct sk_buff *skb, gfp_t allocation, | |
92 | { | |
93 | struct netns_pfkey *net_pfkey = net_generic(net, pfkey_net_id); | |
94 | struct sock *sk; | |
95 | - struct sk_buff *skb2 = NULL; | |
96 | int err = -ESRCH; | |
97 | ||
98 | /* XXX Do we need something like netlink_overrun? I think | |
99 | @@ -253,7 +244,7 @@ static int pfkey_broadcast(struct sk_buff *skb, gfp_t allocation, | |
100 | * socket. | |
101 | */ | |
102 | if (pfk->promisc) | |
103 | - pfkey_broadcast_one(skb, &skb2, GFP_ATOMIC, sk); | |
104 | + pfkey_broadcast_one(skb, GFP_ATOMIC, sk); | |
105 | ||
106 | /* the exact target will be processed later */ | |
107 | if (sk == one_sk) | |
108 | @@ -268,7 +259,7 @@ static int pfkey_broadcast(struct sk_buff *skb, gfp_t allocation, | |
109 | continue; | |
110 | } | |
111 | ||
112 | - err2 = pfkey_broadcast_one(skb, &skb2, GFP_ATOMIC, sk); | |
113 | + err2 = pfkey_broadcast_one(skb, GFP_ATOMIC, sk); | |
114 | ||
115 | /* Error is cleared after successful sending to at least one | |
116 | * registered KM */ | |
117 | @@ -278,9 +269,8 @@ static int pfkey_broadcast(struct sk_buff *skb, gfp_t allocation, | |
118 | rcu_read_unlock(); | |
119 | ||
120 | if (one_sk != NULL) | |
121 | - err = pfkey_broadcast_one(skb, &skb2, allocation, one_sk); | |
122 | + err = pfkey_broadcast_one(skb, allocation, one_sk); | |
123 | ||
124 | - kfree_skb(skb2); | |
125 | kfree_skb(skb); | |
126 | return err; | |
127 | } | |
128 | -- | |
129 | 2.19.1 | |
130 |