On 01.02.2018 21:02, Tommi Rantala wrote:

2018-01-31 19:51 GMT+02:00 Tommi Rantala [off-list ref]:

quoted

On 31.01.2018 14:31, Neil Horman wrote:

quoted

On Wed, Jan 31, 2018 at 11:42:24AM +0200, Tommi Rantala wrote:

quoted

I think there's a problem in the dst refcounting in sctp_v4_get_dst()

There's a dst_entry struct that has >0 refcnt after running the testcase,
which makes it impossible to delete the loopback device, as that dst is
never freed.

I'll try to make a patch.

Are you looking at the second for loop there, which uses
ip_route_output_key,
but discards the result if dst is already set?  That does look a bit
wonky, and
the same problem may exist in the ipv6 path.  Let me know what the result
is.

Yes, that was it. Did you receive the email I sent with the patch?
(I'm not seeing that message e.g. at spinics.net linux-sctp archive, so just
wondering if that email got lost somehow...)

I'll check the ipv6 case, did not try it yet.

Tommi

As far as I can tell, the ipv6 code does not suffer from this.
The dst handling there looks good to me.

For ipv6 part, shouldn't we release 'bdst' there if the previous address
match is better and we continue to the next iteration?

diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
index 5d4c15b..a044096 100644
--- a/net/sctp/ipv6.c
+++ b/net/sctp/ipv6.c

@@ -336,8 +336,11 @@ static void sctp_v6_get_dst(struct sctp_transport *t, union sctp_addr *saddr,
                }
 
                bmatchlen = sctp_v6_addr_match_len(daddr, &laddr->a);
-               if (matchlen > bmatchlen)
+               if (matchlen > bmatchlen) {
+                       if (!IS_ERR(bdst))
+                               dst_release(bdst);
                        continue;
+               }
 
                if (!IS_ERR_OR_NULL(dst))
                        dst_release(dst);

Thanks,
Alexey