* [PATCH 1/1] net: possible use after free in dst_release
@ 2016-01-06 8:18 Francesco Ruggeri
2016-01-06 13:57 ` Eric Dumazet
0 siblings, 1 reply; 3+ messages in thread
From: Francesco Ruggeri @ 2016-01-06 8:18 UTC (permalink / raw)
To: fruggeri, edumazet, davem, netdev
dst_release should not access dst->flags after decrementing
__refcnt to 0. The dst_entry may be in dst_busy_list and
dst_gc_task may dst_destroy it before dst_release gets a chance
to access dst->flags.
Signed-off-by: Francesco Ruggeri <fruggeri@arista.com>
---
net/core/dst.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/core/dst.c b/net/core/dst.c
index e6dc772..a1656e3 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -301,12 +301,13 @@ void dst_release(struct dst_entry *dst)
{
if (dst) {
int newrefcnt;
+ unsigned short nocache = dst->flags & DST_NOCACHE;
newrefcnt = atomic_dec_return(&dst->__refcnt);
if (unlikely(newrefcnt < 0))
net_warn_ratelimited("%s: dst:%p refcnt:%d\n",
__func__, dst, newrefcnt);
- if (!newrefcnt && unlikely(dst->flags & DST_NOCACHE))
+ if (!newrefcnt && unlikely(nocache))
call_rcu(&dst->rcu_head, dst_destroy_rcu);
}
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH 1/1] net: possible use after free in dst_release
2016-01-06 8:18 [PATCH 1/1] net: possible use after free in dst_release Francesco Ruggeri
@ 2016-01-06 13:57 ` Eric Dumazet
2016-01-06 20:00 ` David Miller
0 siblings, 1 reply; 3+ messages in thread
From: Eric Dumazet @ 2016-01-06 13:57 UTC (permalink / raw)
To: Francesco Ruggeri; +Cc: fruggeri, edumazet, davem, netdev
On Wed, 2016-01-06 at 00:18 -0800, Francesco Ruggeri wrote:
> dst_release should not access dst->flags after decrementing
> __refcnt to 0. The dst_entry may be in dst_busy_list and
> dst_gc_task may dst_destroy it before dst_release gets a chance
> to access dst->flags.
>
> Signed-off-by: Francesco Ruggeri <fruggeri@arista.com>
> ---
> net/core/dst.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/core/dst.c b/net/core/dst.c
> index e6dc772..a1656e3 100644
> --- a/net/core/dst.c
> +++ b/net/core/dst.c
> @@ -301,12 +301,13 @@ void dst_release(struct dst_entry *dst)
> {
> if (dst) {
> int newrefcnt;
> + unsigned short nocache = dst->flags & DST_NOCACHE;
>
> newrefcnt = atomic_dec_return(&dst->__refcnt);
> if (unlikely(newrefcnt < 0))
> net_warn_ratelimited("%s: dst:%p refcnt:%d\n",
> __func__, dst, newrefcnt);
> - if (!newrefcnt && unlikely(dst->flags & DST_NOCACHE))
> + if (!newrefcnt && unlikely(nocache))
> call_rcu(&dst->rcu_head, dst_destroy_rcu);
> }
> }
You're right.
My previous fix (commit d69bbf88c8d0) was okay only for DST_NOCACHE dst.
Fixes: d69bbf88c8d0 ("net: fix a race in dst_release()")
Fixes: 27b75c95f10d ("net: avoid RCU for NOCACHE dst")
Acked-by: Eric Dumazet <edumazet@google.com>
Thanks !
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH 1/1] net: possible use after free in dst_release
2016-01-06 13:57 ` Eric Dumazet
@ 2016-01-06 20:00 ` David Miller
0 siblings, 0 replies; 3+ messages in thread
From: David Miller @ 2016-01-06 20:00 UTC (permalink / raw)
To: eric.dumazet; +Cc: fruggeri, fruggeri, edumazet, netdev
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 06 Jan 2016 05:57:24 -0800
> On Wed, 2016-01-06 at 00:18 -0800, Francesco Ruggeri wrote:
>> dst_release should not access dst->flags after decrementing
>> __refcnt to 0. The dst_entry may be in dst_busy_list and
>> dst_gc_task may dst_destroy it before dst_release gets a chance
>> to access dst->flags.
>>
>> Signed-off-by: Francesco Ruggeri <fruggeri@arista.com>
>> ---
>> net/core/dst.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/core/dst.c b/net/core/dst.c
>> index e6dc772..a1656e3 100644
>> --- a/net/core/dst.c
>> +++ b/net/core/dst.c
>> @@ -301,12 +301,13 @@ void dst_release(struct dst_entry *dst)
>> {
>> if (dst) {
>> int newrefcnt;
>> + unsigned short nocache = dst->flags & DST_NOCACHE;
>>
>> newrefcnt = atomic_dec_return(&dst->__refcnt);
>> if (unlikely(newrefcnt < 0))
>> net_warn_ratelimited("%s: dst:%p refcnt:%d\n",
>> __func__, dst, newrefcnt);
>> - if (!newrefcnt && unlikely(dst->flags & DST_NOCACHE))
>> + if (!newrefcnt && unlikely(nocache))
>> call_rcu(&dst->rcu_head, dst_destroy_rcu);
>> }
>> }
>
> You're right.
>
> My previous fix (commit d69bbf88c8d0) was okay only for DST_NOCACHE dst.
>
> Fixes: d69bbf88c8d0 ("net: fix a race in dst_release()")
> Fixes: 27b75c95f10d ("net: avoid RCU for NOCACHE dst")
> Acked-by: Eric Dumazet <edumazet@google.com>
Applied and queued up for -stable, thanks everyone!
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-01-06 20:00 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-01-06 8:18 [PATCH 1/1] net: possible use after free in dst_release Francesco Ruggeri
2016-01-06 13:57 ` Eric Dumazet
2016-01-06 20:00 ` 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).