From: David Laight
> Sent: 14 August 2020 17:18
>
> > > > At some point the negotiation of the number of SCTP streams
> > > > seems to have got broken.
> > > > I've definitely tested it in the past (probably 10 years ago!)
> > > > but on a 5.8.0 kernel getsockopt(SCTP_INFO) seems to be
> > > > returning the 'num_ostreams' set by setsockopt(SCTP_INIT)
> > > > rather than the smaller of that value and that configured
> > > > at the other end of the connection.
> > > >
> > > > I'll do a bit of digging.
> > >
> > > I can't find the code that processes the init_ack.
> > > But when sctp_procss_int() saves the smaller value
> > > in asoc->c.sinint_max_ostreams.
> > >
> > > But afe899962ee079 (if I've typed it right) changed
> > > the values SCTP_INFO reported.
> > > Apparantly adding 'sctp reconfig' had changed things.
> > >
> > > So I suspect this has all been broken for over 3 years.
> >
> > It looks like the changes that broke it went into 4.11.
> > I've just checked a 3.8 kernel and that negotiates the
> > values down in both directions.
> >
> > I don't have any kernels lurking between 3.8 and 4.15.
> > (Yes, I could build one, but it doesn't really help.)
>
> Ok, bug located - pretty obvious really.
> net/sctp/stream. has the following code:
>
> static int sctp_stream_alloc_out(struct sctp_stream *stream, __u16 outcnt,
> gfp_t gfp)
> {
> int ret;
>
> if (outcnt <= stream->outcnt)
> return 0;
Deleting this check is sufficient to fix the code.
Along with the equivalent check in sctp_stream-alloc_in().
> This does mean that it has only been broken since the 5.1
> merge window.
And is a good candidate for the back-ports.
> ret = genradix_prealloc(&stream->out, outcnt, gfp);
> if (ret)
> return ret;
>
> stream->outcnt = outcnt;
> return 0;
> }
>
> sctp_stream_alloc_in() is the same.
>
> This is called to reduce the number of streams.
> But in that case it does nothing at all.
>
> Which means that the 'convert to genradix' change broke it.
> Tag 2075e50caf5ea.
>
> I don't know what 'genradix' arrays or the earlier 'flex_array'
> actually look like.
> But if 'genradix' is some kind of radix-tree it is probably the
> wrong beast for SCTP streams.
> Lots of code loops through all of them.
Yep, I'm pretty sure a kvmalloc() would be best.
> While just assigning to stream->outcnt when the value
> is reduced will fix the negotiation, I've no idea
> what side-effects that has.
I've done some checks.
The arrays are allocated when an INIT is sent and also before
a received INIT is processed.
So if one side (eg the responder) allocates a very big value
then the associated memory is never freed when the value
is negotiated down.
There is a comment to the effect that this is desirable.
If my quick calculations are correct then each 'in' is 20 bytes
and each 'out' 24 (with a lot of pad bytes).
So the max sizes are 322 and 386 4k pages.
I haven't looked at how many of the 'out' streams gets the
extra, separately allocated, structure.
I suspect the memory footprint for a single SCTP connection
is potentially huge.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
