netdev - RE: sctp: num_ostreams and max

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0c1621e5da2e41e8905762d0208f9d40@AcuMS.aculab.com>
Date:   Sat, 15 Aug 2020 14:49:31 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     "'linux-sctp@...r.kernel.org'" <linux-sctp@...r.kernel.org>,
        'Neil Horman' <nhorman@...driver.com>,
        "'kent.overstreet@...il.com'" <kent.overstreet@...il.com>,
        'Andrew Morton' <akpm@...ux-foundation.org>
CC:     "'netdev@...r.kernel.org'" <netdev@...r.kernel.org>
Subject: RE: sctp: num_ostreams and max_instreams negotiation

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)