[PATCH] net: sctp: Fix negotiation of the number of data streams.

[PATCH] net: sctp: Fix negotiation of the number of data streams.

[David Laight]

The number of streams offered by the remote system was being ignored.
Any data sent on those streams would get discarded by the remote system.

Fixes 2075e50caf5ea.

Signed-off-by: David Laight <david.laight@aculab.com>
---
 net/sctp/stream.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

This needs backporting to 5.1 and all later kernels.

diff --git a/net/sctp/stream.c b/net/sctp/stream.c
index bda2536dd740..6dc95dcc0ff4 100644
--- a/net/sctp/stream.c
+++ b/net/sctp/stream.c
@@ -88,12 +88,13 @@ static int sctp_stream_alloc_out(struct sctp_stream *stream, __u16 outcnt,
 	int ret;
 
 	if (outcnt <= stream->outcnt)
-		return 0;
+		goto out;
 
 	ret = genradix_prealloc(&stream->out, outcnt, gfp);
 	if (ret)
 		return ret;
 
+out:
 	stream->outcnt = outcnt;
 	return 0;
 }
@@ -104,12 +105,13 @@ static int sctp_stream_alloc_in(struct sctp_stream *stream, __u16 incnt,
 	int ret;
 
 	if (incnt <= stream->incnt)
-		return 0;
+		goto out;
 
 	ret = genradix_prealloc(&stream->in, incnt, gfp);
 	if (ret)
 		return ret;
 
+out:
 	stream->incnt = incnt;
 	return 0;
 }
-- 
2.25.1

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH] net: sctp: Fix negotiation of the number of data streams.

[David Miller]

From: David Laight <David.Laight@ACULAB.COM>
Date: Tue, 18 Aug 2020 14:36:58 +0000

> Fixes 2075e50caf5ea.
> 
> Signed-off-by: David Laight <david.laight@aculab.com>

This is not the correct format for a Fixes tag, also it should not
be separated by empty lines from other tags such as Signed-off-by
and Acked-by.

As someone who reads netdev frequently, these patterns should be
etched into your brain by now. :-)

RE: [PATCH] net: sctp: Fix negotiation of the number of data streams.

[David Laight]

From: David Miller
> Sent: 18 August 2020 20:58
> 
> From: David Laight <David.Laight@ACULAB.COM>
> Date: Tue, 18 Aug 2020 14:36:58 +0000
> 
> > Fixes 2075e50caf5ea.
> >
> > Signed-off-by: David Laight <david.laight@aculab.com>
> 
> This is not the correct format for a Fixes tag, also it should not
> be separated by empty lines from other tags such as Signed-off-by
> and Acked-by.
> 
> As someone who reads netdev frequently, these patterns should be
> etched into your brain by now. :-)

Sorry I don't often write patches - I'm only paid to support
our drivers, not fix the Linux kernel :-)
(We don't ship a kernel, our drivers have to work in disto kernels.)

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH] net: sctp: Fix negotiation of the number of data streams.

['Marcelo Ricardo Leitner']

On Tue, Aug 18, 2020 at 02:36:58PM +0000, David Laight wrote:
> The number of streams offered by the remote system was being ignored.
> Any data sent on those streams would get discarded by the remote system.

That's quite brief and not accurate: it was only ignored if 'Xcnt <stream->Xcnt'.

Other than this and the Fixes tag, LGTM. Passes the tests here. I'll
ack the v2 then.

> 
> Fixes 2075e50caf5ea.
> 
> Signed-off-by: David Laight <david.laight@aculab.com>
> ---
>  net/sctp/stream.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> This needs backporting to 5.1 and all later kernels.

A 'net' tree tag in patch tags is welcomed.

> 
> diff --git a/net/sctp/stream.c b/net/sctp/stream.c
> index bda2536dd740..6dc95dcc0ff4 100644
> --- a/net/sctp/stream.c
> +++ b/net/sctp/stream.c
> @@ -88,12 +88,13 @@ static int sctp_stream_alloc_out(struct sctp_stream *stream, __u16 outcnt,
>  	int ret;
>  
>  	if (outcnt <= stream->outcnt)
> -		return 0;
> +		goto out;
>  
>  	ret = genradix_prealloc(&stream->out, outcnt, gfp);
>  	if (ret)
>  		return ret;
>  
> +out:
>  	stream->outcnt = outcnt;
>  	return 0;
>  }
> @@ -104,12 +105,13 @@ static int sctp_stream_alloc_in(struct sctp_stream *stream, __u16 incnt,
>  	int ret;
>  
>  	if (incnt <= stream->incnt)
> -		return 0;
> +		goto out;
>  
>  	ret = genradix_prealloc(&stream->in, incnt, gfp);
>  	if (ret)
>  		return ret;
>  
> +out:
>  	stream->incnt = incnt;
>  	return 0;
>  }
> -- 
> 2.25.1
> 
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
> 

RE: [PATCH] net: sctp: Fix negotiation of the number of data streams.

[David Laight]

From: 'Marcelo Ricardo Leitner'
> Sent: 18 August 2020 22:47
> 
> On Tue, Aug 18, 2020 at 02:36:58PM +0000, David Laight wrote:
> > The number of streams offered by the remote system was being ignored.
> > Any data sent on those streams would get discarded by the remote system.
> 
> That's quite brief and not accurate: it was only ignored if 'Xcnt <> stream->Xcnt'.

The number of streams (esp out ones) received from the remote
system in an INIT or INIT_ACK was ignored.
So it would always send data chunks using the number of streams
requested by the local user.
I managed to tweak our M3UA config to get invalid stream numbers
sent on both inwards and outwards connections.

I only noticed because of testing a (slightly horrid) workaround
for no longer being able to use kernel_getsockopt() to retrieve
the number of ostreams.
The number of ostreams was about the only thing we didn't trace :-(
At least my code can now obtain the correct value even for buggy
kernels.

> Other than this and the Fixes tag, LGTM. Passes the tests here. I'll
> ack the v2 then.

I wasn't sure whether DM actually wanted a V2 with the fixes
tag fixed.
I can send one tomorrow.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)