| From foo@baz Thu Jun 29 19:45:34 CEST 2017 |
| From: Wei Wang <weiwan@google.com> |
| Date: Fri, 16 Jun 2017 10:46:37 -0700 |
| Subject: decnet: always not take dst->__refcnt when inserting dst into hash table |
| |
| From: Wei Wang <weiwan@google.com> |
| |
| |
| [ Upstream commit 76371d2e3ad1f84426a30ebcd8c3b9b98f4c724f ] |
| |
| In the existing dn_route.c code, dn_route_output_slow() takes |
| dst->__refcnt before calling dn_insert_route() while dn_route_input_slow() |
| does not take dst->__refcnt before calling dn_insert_route(). |
| This makes the whole routing code very buggy. |
| In dn_dst_check_expire(), dnrt_free() is called when rt expires. This |
| makes the routes inserted by dn_route_output_slow() not able to be |
| freed as the refcnt is not released. |
| In dn_dst_gc(), dnrt_drop() is called to release rt which could |
| potentially cause the dst->__refcnt to be dropped to -1. |
| In dn_run_flush(), dst_free() is called to release all the dst. Again, |
| it makes the dst inserted by dn_route_output_slow() not able to be |
| released and also, it does not wait on the rcu and could potentially |
| cause crash in the path where other users still refer to this dst. |
| |
| This patch makes sure both input and output path do not take |
| dst->__refcnt before calling dn_insert_route() and also makes sure |
| dnrt_free()/dst_free() is called when removing dst from the hash table. |
| The only difference between those 2 calls is that dnrt_free() waits on |
| the rcu while dst_free() does not. |
| |
| Signed-off-by: Wei Wang <weiwan@google.com> |
| Acked-by: Martin KaFai Lau <kafai@fb.com> |
| Signed-off-by: David S. Miller <davem@davemloft.net> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| net/decnet/dn_route.c | 14 ++++---------- |
| 1 file changed, 4 insertions(+), 10 deletions(-) |
| |
| --- a/net/decnet/dn_route.c |
| +++ b/net/decnet/dn_route.c |
| @@ -189,12 +189,6 @@ static inline void dnrt_free(struct dn_r |
| call_rcu_bh(&rt->dst.rcu_head, dst_rcu_free); |
| } |
| |
| -static inline void dnrt_drop(struct dn_route *rt) |
| -{ |
| - dst_release(&rt->dst); |
| - call_rcu_bh(&rt->dst.rcu_head, dst_rcu_free); |
| -} |
| - |
| static void dn_dst_check_expire(unsigned long dummy) |
| { |
| int i; |
| @@ -249,7 +243,7 @@ static int dn_dst_gc(struct dst_ops *ops |
| } |
| *rtp = rt->dst.dn_next; |
| rt->dst.dn_next = NULL; |
| - dnrt_drop(rt); |
| + dnrt_free(rt); |
| break; |
| } |
| spin_unlock_bh(&dn_rt_hash_table[i].lock); |
| @@ -351,7 +345,7 @@ static int dn_insert_route(struct dn_rou |
| dst_use(&rth->dst, now); |
| spin_unlock_bh(&dn_rt_hash_table[hash].lock); |
| |
| - dnrt_drop(rt); |
| + dst_free(&rt->dst); |
| *rp = rth; |
| return 0; |
| } |
| @@ -381,7 +375,7 @@ static void dn_run_flush(unsigned long d |
| for(; rt; rt = next) { |
| next = rcu_dereference_raw(rt->dst.dn_next); |
| RCU_INIT_POINTER(rt->dst.dn_next, NULL); |
| - dst_free((struct dst_entry *)rt); |
| + dnrt_free(rt); |
| } |
| |
| nothing_to_declare: |
| @@ -1195,7 +1189,7 @@ make_route: |
| if (dev_out->flags & IFF_LOOPBACK) |
| flags |= RTCF_LOCAL; |
| |
| - rt = dst_alloc(&dn_dst_ops, dev_out, 1, DST_OBSOLETE_NONE, DST_HOST); |
| + rt = dst_alloc(&dn_dst_ops, dev_out, 0, DST_OBSOLETE_NONE, DST_HOST); |
| if (rt == NULL) |
| goto e_nobufs; |
| |