* [PATCH] ipv6: fix a potential use after free in sit.c
@ 2014-10-18 9:33 roy.qing.li
2014-10-18 15:52 ` Eric Dumazet
0 siblings, 1 reply; 5+ messages in thread
From: roy.qing.li @ 2014-10-18 9:33 UTC (permalink / raw)
To: netdev; +Cc: ghorbel
From: Li RongQing <roy.qing.li@gmail.com>
pskb_may_pull() maybe change skb->data and make iph pointer oboslete,
fix it by geting ip header length directly.
Fixes: ca15a078 (sit: generate icmpv6 error when receiving icmpv4 error)
Cc: Oussama Ghorbel <ghorbel@pivasoftware.com>
Signed-off-by: Li RongQing <roy.qing.li@gmail.com>
---
net/ipv6/sit.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index 6eab37c..0ccadab 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -485,11 +485,11 @@ static void ipip6_tunnel_uninit(struct net_device *dev)
*/
static int ipip6_err_gen_icmpv6_unreach(struct sk_buff *skb)
{
- const struct iphdr *iph = (const struct iphdr *) skb->data;
+ int ihl = ((const struct iphdr *)skb->data)->ihl*4;
struct rt6_info *rt;
struct sk_buff *skb2;
- if (!pskb_may_pull(skb, iph->ihl * 4 + sizeof(struct ipv6hdr) + 8))
+ if (!pskb_may_pull(skb, ihl + sizeof(struct ipv6hdr) + 8))
return 1;
skb2 = skb_clone(skb, GFP_ATOMIC);
@@ -498,7 +498,7 @@ static int ipip6_err_gen_icmpv6_unreach(struct sk_buff *skb)
return 1;
skb_dst_drop(skb2);
- skb_pull(skb2, iph->ihl * 4);
+ skb_pull(skb2, ihl);
skb_reset_network_header(skb2);
rt = rt6_lookup(dev_net(skb->dev), &ipv6_hdr(skb2)->saddr, NULL, 0, 0);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] ipv6: fix a potential use after free in sit.c
2014-10-18 9:33 [PATCH] ipv6: fix a potential use after free in sit.c roy.qing.li
@ 2014-10-18 15:52 ` Eric Dumazet
2014-10-18 17:07 ` David Miller
0 siblings, 1 reply; 5+ messages in thread
From: Eric Dumazet @ 2014-10-18 15:52 UTC (permalink / raw)
To: roy.qing.li; +Cc: netdev, ghorbel
On Sat, 2014-10-18 at 17:33 +0800, roy.qing.li@gmail.com wrote:
> From: Li RongQing <roy.qing.li@gmail.com>
>
> pskb_may_pull() maybe change skb->data and make iph pointer oboslete,
> fix it by geting ip header length directly.
>
> Fixes: ca15a078 (sit: generate icmpv6 error when receiving icmpv4 error)
> Cc: Oussama Ghorbel <ghorbel@pivasoftware.com>
> Signed-off-by: Li RongQing <roy.qing.li@gmail.com>
> ---
> net/ipv6/sit.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
Acked-by: Eric Dumazet <edumazet@google.com>
Thanks for doing all these checks !
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ipv6: fix a potential use after free in sit.c
2014-10-18 15:52 ` Eric Dumazet
@ 2014-10-18 17:07 ` David Miller
2014-10-18 17:22 ` Dave Taht
0 siblings, 1 reply; 5+ messages in thread
From: David Miller @ 2014-10-18 17:07 UTC (permalink / raw)
To: eric.dumazet; +Cc: roy.qing.li, netdev, ghorbel
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sat, 18 Oct 2014 08:52:20 -0700
> On Sat, 2014-10-18 at 17:33 +0800, roy.qing.li@gmail.com wrote:
>> From: Li RongQing <roy.qing.li@gmail.com>
>>
>> pskb_may_pull() maybe change skb->data and make iph pointer oboslete,
>> fix it by geting ip header length directly.
>>
>> Fixes: ca15a078 (sit: generate icmpv6 error when receiving icmpv4 error)
>> Cc: Oussama Ghorbel <ghorbel@pivasoftware.com>
>> Signed-off-by: Li RongQing <roy.qing.li@gmail.com>
>> ---
>> net/ipv6/sit.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> Acked-by: Eric Dumazet <edumazet@google.com>
>
> Thanks for doing all these checks !
Indeed, I wish we could somehow automate this.
Yes, I'm sure we could construct some sparse et al. rules but
I mean at run time. For example, having a special pointer type
that you can't dereference directly. At the time of assignment
the pointer gets some kind of state, and pskb_may_pull() et al.
calls invalidate that "state".
It probably could just be a 2-bit counter which is incremented
every time skb->data is reallocated.
The captured 2-bit generation count could be stored in the low
bits of the pointer.
Anyways, just throwing out ideas...
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ipv6: fix a potential use after free in sit.c
2014-10-18 17:07 ` David Miller
@ 2014-10-18 17:22 ` Dave Taht
2014-10-18 17:36 ` David Miller
0 siblings, 1 reply; 5+ messages in thread
From: Dave Taht @ 2014-10-18 17:22 UTC (permalink / raw)
To: David Miller; +Cc: Eric Dumazet, roy.qing.li, netdev, ghorbel
On Sat, Oct 18, 2014 at 10:07 AM, David Miller <davem@davemloft.net> wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Indeed, I wish we could somehow automate this.
>
> Yes, I'm sure we could construct some sparse et al. rules but
> I mean at run time. For example, having a special pointer type
> that you can't dereference directly. At the time of assignment
> the pointer gets some kind of state, and pskb_may_pull() et al.
> calls invalidate that "state".
>
> It probably could just be a 2-bit counter which is incremented
> every time skb->data is reallocated.
>
> The captured 2-bit generation count could be stored in the low
> bits of the pointer.
>
> Anyways, just throwing out ideas...
So far as I know, the one machine linux has never been ported
to was a LISP machine. With its 36 bit words, and tagging, that
would do what you want. I had kind of hoped with 64 bits of
addressing some of those ideas would have been reused.
/me misses truly oddball architectures
--
Dave Täht
http://www.bufferbloat.net/projects/bloat/wiki/Upcoming_Talks
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ipv6: fix a potential use after free in sit.c
2014-10-18 17:22 ` Dave Taht
@ 2014-10-18 17:36 ` David Miller
0 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2014-10-18 17:36 UTC (permalink / raw)
To: dave.taht; +Cc: eric.dumazet, roy.qing.li, netdev, ghorbel
From: Dave Taht <dave.taht@gmail.com>
Date: Sat, 18 Oct 2014 10:22:35 -0700
> So far as I know, the one machine linux has never been ported
> to was a LISP machine. With its 36 bit words, and tagging, that
> would do what you want. I had kind of hoped with 64 bits of
> addressing some of those ideas would have been reused.
>
> /me misses truly oddball architectures
Sparc cpus have tagged add and subtract instructions. :)
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-10-18 17:36 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-18 9:33 [PATCH] ipv6: fix a potential use after free in sit.c roy.qing.li
2014-10-18 15:52 ` Eric Dumazet
2014-10-18 17:07 ` David Miller
2014-10-18 17:22 ` Dave Taht
2014-10-18 17:36 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).