diff options
author | Herbert Xu <herbert@gondor.apana.org.au> | 2015-08-04 03:42:47 -0400 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2015-08-07 00:55:47 -0400 |
commit | a0a2a6602496a45ae838a96db8b8173794b5d398 (patch) | |
tree | 5508d9f02066c30854b0c88b82fffc03c857adef /net | |
parent | 14d2b7c1a96ef37eb571599c73d4a1a606b964d6 (diff) |
net: Fix skb_set_peeked use-after-free bug
The commit 738ac1ebb96d02e0d23bc320302a6ea94c612dec ("net: Clone
skb before setting peeked flag") introduced a use-after-free bug
in skb_recv_datagram. This is because skb_set_peeked may create
a new skb and free the existing one. As it stands the caller will
continue to use the old freed skb.
This patch fixes it by making skb_set_peeked return the new skb
(or the old one if unchanged).
Fixes: 738ac1ebb96d ("net: Clone skb before setting peeked flag")
Reported-by: Brenden Blanco <bblanco@plumgrid.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Tested-by: Brenden Blanco <bblanco@plumgrid.com>
Reviewed-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'net')
-rw-r--r-- | net/core/datagram.c | 13 |
1 files changed, 7 insertions, 6 deletions
diff --git a/net/core/datagram.c b/net/core/datagram.c index 4967262b2707..617088aee21d 100644 --- a/net/core/datagram.c +++ b/net/core/datagram.c | |||
@@ -131,12 +131,12 @@ out_noerr: | |||
131 | goto out; | 131 | goto out; |
132 | } | 132 | } |
133 | 133 | ||
134 | static int skb_set_peeked(struct sk_buff *skb) | 134 | static struct sk_buff *skb_set_peeked(struct sk_buff *skb) |
135 | { | 135 | { |
136 | struct sk_buff *nskb; | 136 | struct sk_buff *nskb; |
137 | 137 | ||
138 | if (skb->peeked) | 138 | if (skb->peeked) |
139 | return 0; | 139 | return skb; |
140 | 140 | ||
141 | /* We have to unshare an skb before modifying it. */ | 141 | /* We have to unshare an skb before modifying it. */ |
142 | if (!skb_shared(skb)) | 142 | if (!skb_shared(skb)) |
@@ -144,7 +144,7 @@ static int skb_set_peeked(struct sk_buff *skb) | |||
144 | 144 | ||
145 | nskb = skb_clone(skb, GFP_ATOMIC); | 145 | nskb = skb_clone(skb, GFP_ATOMIC); |
146 | if (!nskb) | 146 | if (!nskb) |
147 | return -ENOMEM; | 147 | return ERR_PTR(-ENOMEM); |
148 | 148 | ||
149 | skb->prev->next = nskb; | 149 | skb->prev->next = nskb; |
150 | skb->next->prev = nskb; | 150 | skb->next->prev = nskb; |
@@ -157,7 +157,7 @@ static int skb_set_peeked(struct sk_buff *skb) | |||
157 | done: | 157 | done: |
158 | skb->peeked = 1; | 158 | skb->peeked = 1; |
159 | 159 | ||
160 | return 0; | 160 | return skb; |
161 | } | 161 | } |
162 | 162 | ||
163 | /** | 163 | /** |
@@ -229,8 +229,9 @@ struct sk_buff *__skb_recv_datagram(struct sock *sk, unsigned int flags, | |||
229 | continue; | 229 | continue; |
230 | } | 230 | } |
231 | 231 | ||
232 | error = skb_set_peeked(skb); | 232 | skb = skb_set_peeked(skb); |
233 | if (error) | 233 | error = PTR_ERR(skb); |
234 | if (IS_ERR(skb)) | ||
234 | goto unlock_err; | 235 | goto unlock_err; |
235 | 236 | ||
236 | atomic_inc(&skb->users); | 237 | atomic_inc(&skb->users); |