From: Vlad Yasevich <vyasevich@gmail.com>
To: David Laight <David.Laight@ACULAB.COM>,
Linux Networking Development Mailing List
<netdev@vger.kernel.org>,
"'linux-sctp@vger.kernel.org'" <linux-sctp@vger.kernel.org>
Cc: David Miller <davem@davemloft.net>
Subject: Re: [PATCH net-next v3 3/3] net: sctp: Add support for MSG_MORE on SCTP
Date: Tue, 22 Jul 2014 16:38:17 +0000 [thread overview]
Message-ID: <53CE9379.4030500@gmail.com> (raw)
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D1727B3E7@AcuExch.aculab.com>
On 07/22/2014 11:22 AM, David Laight wrote:
> ...
>>> All this is really defining the behaviour for 'broken' apps.
>>> It wouldn't be completely unreasonable if a single message sent with 'MSG_MORE'
>>> never actually got sent.
>>
>> Actually, this code opens up a way to eat kernel memory. Consider an application
>> that does:
>> while (1) {
>> socket(IPPROTO_SCTP);
>> connect()
>> sendmsg(MSG_MORE); <- write 1 byte
>> close();
>> }
>>
>> Because we send with MSG_MORE, the 1 byte gets queued to the association. The close()
>> causes us to enter SHUTDOWN_PENDING state and we never flush the buffer and close
>> the association.
>>
>> This is a malicious example. Similarly, a broken application could just forget to
>> clear MSG_MORE and when we end up in a condition where the amount of queued data is
>> smaller then MTU and all inflight data has been acked, we'd once again be stuck.
>
> From a system point of view that one doesn't really matter.
>
>> Just because application isn't doing the right thing, we can't assume it's broken. It
>> could be malicious. We need to make this behavior robust.
>
> Hmmm....
> The amount of data per-socket is limited by the mtu and socket buffer size,
> so a malicious app would have to create a lot of sockets to use a significant
> amount of kernel memory.
it's not just the data, all the corresponding data structures which are particularly
fat in sctp.
>
> If close() blocks forever, isn't there a worse problem with a remote app
> that causes the window to be zero - and so data can't be sent and the
> close will block forever anyway?
No, this doesn't happen since there are triggered as retransmissions and
once we enter SHUTDOWN-PENDING, we can start the shutdown-guard timer
once we've hit the max retransmit count. This solves this issue.
> I've never liked protocols that wait forever for send data to drain on disconnect.
> Seems like an 'accident' waiting to happen.
>
> I'm not sure what would be needed to be done to cause queued send data to be
> sent when SCTP_F_TX_MSG_MORE is cleared by close() or set/clear SCTP_NODELAY.
> It is probably a simple call to the correct function.
OK. I'll try to come up with some code for this.
-vlad
>
> David
>
>
>
>
WARNING: multiple messages have this Message-ID (diff)
From: Vlad Yasevich <vyasevich@gmail.com>
To: David Laight <David.Laight@ACULAB.COM>,
Linux Networking Development Mailing List
<netdev@vger.kernel.org>,
"'linux-sctp@vger.kernel.org'" <linux-sctp@vger.kernel.org>
Cc: David Miller <davem@davemloft.net>
Subject: Re: [PATCH net-next v3 3/3] net: sctp: Add support for MSG_MORE on SCTP
Date: Tue, 22 Jul 2014 12:38:17 -0400 [thread overview]
Message-ID: <53CE9379.4030500@gmail.com> (raw)
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D1727B3E7@AcuExch.aculab.com>
On 07/22/2014 11:22 AM, David Laight wrote:
> ...
>>> All this is really defining the behaviour for 'broken' apps.
>>> It wouldn't be completely unreasonable if a single message sent with 'MSG_MORE'
>>> never actually got sent.
>>
>> Actually, this code opens up a way to eat kernel memory. Consider an application
>> that does:
>> while (1) {
>> socket(IPPROTO_SCTP);
>> connect()
>> sendmsg(MSG_MORE); <- write 1 byte
>> close();
>> }
>>
>> Because we send with MSG_MORE, the 1 byte gets queued to the association. The close()
>> causes us to enter SHUTDOWN_PENDING state and we never flush the buffer and close
>> the association.
>>
>> This is a malicious example. Similarly, a broken application could just forget to
>> clear MSG_MORE and when we end up in a condition where the amount of queued data is
>> smaller then MTU and all inflight data has been acked, we'd once again be stuck.
>
> From a system point of view that one doesn't really matter.
>
>> Just because application isn't doing the right thing, we can't assume it's broken. It
>> could be malicious. We need to make this behavior robust.
>
> Hmmm....
> The amount of data per-socket is limited by the mtu and socket buffer size,
> so a malicious app would have to create a lot of sockets to use a significant
> amount of kernel memory.
it's not just the data, all the corresponding data structures which are particularly
fat in sctp.
>
> If close() blocks forever, isn't there a worse problem with a remote app
> that causes the window to be zero - and so data can't be sent and the
> close will block forever anyway?
No, this doesn't happen since there are triggered as retransmissions and
once we enter SHUTDOWN-PENDING, we can start the shutdown-guard timer
once we've hit the max retransmit count. This solves this issue.
> I've never liked protocols that wait forever for send data to drain on disconnect.
> Seems like an 'accident' waiting to happen.
>
> I'm not sure what would be needed to be done to cause queued send data to be
> sent when SCTP_F_TX_MSG_MORE is cleared by close() or set/clear SCTP_NODELAY.
> It is probably a simple call to the correct function.
OK. I'll try to come up with some code for this.
-vlad
>
> David
>
>
>
>
next prev parent reply other threads:[~2014-07-22 16:38 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-07-22 8:59 [PATCH net-next v3 3/3] net: sctp: Add support for MSG_MORE on SCTP David Laight
2014-07-22 8:59 ` David Laight
2014-07-22 13:23 ` Vlad Yasevich
2014-07-22 13:23 ` Vlad Yasevich
2014-07-22 13:45 ` David Laight
2014-07-22 14:54 ` Vlad Yasevich
2014-07-22 14:54 ` Vlad Yasevich
2014-07-22 15:22 ` David Laight
2014-07-22 15:22 ` David Laight
2014-07-22 16:38 ` Vlad Yasevich [this message]
2014-07-22 16:38 ` Vlad Yasevich
2014-07-22 16:46 ` David Laight
2014-07-22 15:25 ` David Laight
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=53CE9379.4030500@gmail.com \
--to=vyasevich@gmail.com \
--cc=David.Laight@ACULAB.COM \
--cc=davem@davemloft.net \
--cc=linux-sctp@vger.kernel.org \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.