10:54 p.m.
Do not process incoming options if the subflow is not MPTCP.
Observed crash when running against out-of-tree kernel with
mis-matched checksum options.
Peter Krystad (2):
mptcp: Ignore incoming MPTCP options when not MP_CAPABLE
mptcp: Ignore incoming MPTCP options when not MP_CAPABLE
net/mptcp/options.c | 3 +++
1 file changed, 3 insertions(+)
--
2.17.2
10:54 p.m.
New subject: [PATCH 1/2] mptcp: Ignore incoming MPTCP options when not MP_CAPABLE
Any options received on a fallback subflow should be ignored.
squashto: Implement MPTCP receive path
Signed-off-by: Peter Krystad <peter.krystad(a)linux.intel.com>
---
net/mptcp/options.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 8f199039971f..dcd7273e488c 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -591,6 +591,9 @@ void mptcp_incoming_options(struct sock *sk, struct sk_buff *skb,
struct mptcp_options_received *mp_opt;
struct mptcp_ext *mpext;
+ if (!subflow->mp_capable)
+ return;
+
mp_opt = &opt_rx->mptcp;
if (msk && mp_opt->add_addr) {
--
2.17.2
10:54 p.m.
New subject: [PATCH 2/2] mptcp: Ignore incoming MPTCP options when not MP_CAPABLE
Any options received on a fallback subflow should be ignored.
squashto: Add handling of incoming MP_JOIN requests
Signed-off-by: Peter Krystad <peter.krystad(a)linux.intel.com>
---
net/mptcp/options.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index dcd7273e488c..1b08e1193991 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -591,7 +591,7 @@ void mptcp_incoming_options(struct sock *sk, struct sk_buff *skb,
struct mptcp_options_received *mp_opt;
struct mptcp_ext *mpext;
- if (!subflow->mp_capable)
+ if (!subflow->mp_capable && !subflow->mp_join)
return;
mp_opt = &opt_rx->mptcp;
--
2.17.2
12:19 p.m.
Hi Peter,
On 23/10/2019 00:54, Peter Krystad wrote:
Observed crash when running against out-of-tree kernel with
mis-matched checksum options.
Thank you for doing this kind of tests!
Do not process incoming options if the subflow is not MPTCP.
Are we not supposed not to deal with MPTCP options after a fallback to TCP?
If we fallback to TCP, should we not always set 'tp->is_mptcp' to false?
Cheers,
Matt
--
Matthieu Baerts | R&D Engineer
matthieu.baerts(a)tessares.net
Tessares SA | Hybrid Access Solutions
www.tessares.net
1 Avenue Jean Monnet, 1348 Louvain-la-Neuve, Belgium
8:35 p.m.
On Wed, 2019-10-23 at 14:19 +0200, Matthieu Baerts wrote:
Hi Peter,
On 23/10/2019 00:54, Peter Krystad wrote:
> Observed crash when running against out-of-tree kernel with
> mis-matched checksum options.
Thank you for doing this kind of tests!
It was an accident, I forgot to disable checksum on the out-of-tree side...
> Do not process incoming options if the subflow is not MPTCP.
Are we not supposed not to deal with MPTCP options after a fallback to TCP?
If we fallback to TCP, should we not always set 'tp->is_mptcp' to false?
Currently for fallback we set is_mptcp=0 after the connection negotiation is
complete [in mptcp_accept()] and presumeably everything is fine after that
point. In this specific situation (we don't send MP_CAPABLE in the third ack
because we don't support checksum) the out-of-tree kernel resends the syn-ack
with DSS options, and these packets are processed before is_mptcp is cleared.
I think we are in uncharted territory here because of how we reject the other
sides request for checksums, but this check can't hurt.
Peter.
Cheers,
Matt
11:40 p.m.
On Tue, 22 Oct 2019, Peter Krystad wrote:
Do not process incoming options if the subflow is not MPTCP.
Observed crash when running against out-of-tree kernel with
mis-matched checksum options.
Peter Krystad (2):
mptcp: Ignore incoming MPTCP options when not MP_CAPABLE
mptcp: Ignore incoming MPTCP options when not MP_CAPABLE
net/mptcp/options.c | 3 +++
1 file changed, 3 insertions(+)
Both changes look good to me. Thanks, Peter!
--
Mat Martineau
Intel
8:04 a.m.
Hi Peter, Mat,
On 26/10/2019 01:40, Mat Martineau wrote:
On Tue, 22 Oct 2019, Peter Krystad wrote:
> Do not process incoming options if the subflow is not MPTCP.
>
> Observed crash when running against out-of-tree kernel with
> mis-matched checksum options.
>
> Peter Krystad (2):
> mptcp: Ignore incoming MPTCP options when not MP_CAPABLE
> mptcp: Ignore incoming MPTCP options when not MP_CAPABLE
>
> net/mptcp/options.c | 3 +++
> 1 file changed, 3 insertions(+)
>
Both changes look good to me. Thanks, Peter!
Thank you for the patches and the reviews!
- 6eb607af7475: "squashed" patch 1/2 in "mptcp: Implement MPTCP receive
path"
- ea443b296417: "squashed" patch 2/2 in "mptcp: Add handling of outgoing
MP_JOIN requests"
- 22b6803a0c48..a89f2feb5b90: result
Tests are still OK!
Cheers,
Matt
--
Matthieu Baerts | R&D Engineer
matthieu.baerts(a)tessares.net
Tessares SA | Hybrid Access Solutions
www.tessares.net
1 Avenue Jean Monnet, 1348 Louvain-la-Neuve, Belgium
3:34 p.m.
Hi,
It seems this email I wanted to send end of last week didn't go through.
On 26/10/2019 10:04, Matthieu Baerts wrote:
Hi Peter, Mat,
On 26/10/2019 01:40, Mat Martineau wrote:
>
> On Tue, 22 Oct 2019, Peter Krystad wrote:
>
>> Do not process incoming options if the subflow is not MPTCP.
>>
>> Observed crash when running against out-of-tree kernel with
>> mis-matched checksum options.
>>
>> Peter Krystad (2):
>> mptcp: Ignore incoming MPTCP options when not MP_CAPABLE
>> mptcp: Ignore incoming MPTCP options when not MP_CAPABLE
>>
>> net/mptcp/options.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>
> Both changes look good to me. Thanks, Peter!
Thank you for the patches and the reviews!
- 6eb607af7475: "squashed" patch 1/2 in "mptcp: Implement MPTCP receive
path"
- ea443b296417: "squashed" patch 2/2 in "mptcp: Add handling of outgoing
MP_JOIN requests"
- 22b6803a0c48..a89f2feb5b90: result
Tests are still OK!
I had to do a small change to allow the compilation of each commit:
diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index ca589ab9a02a..bbf9e9aa53da 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -391,6 +391,7 @@ bool mptcp_synack_options(const struct request_sock
*req, unsigned int *size,
void mptcp_incoming_options(struct sock *sk, struct sk_buff *skb,
struct tcp_options_received *opt_rx)
{
+ struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk);
struct mptcp_options_received *mp_opt;
struct mptcp_ext *mpext;
The subflow variable is declared later on in "mptcp: Add path manager
interface"
- c559c9eb4f9f: "squashed" in "mptcp: Implement MPTCP receive path"
- 9c984d940c41: conflict in t/mptcp-Add-path-manager-interface
- no diff at the end.
Cheers,
Matt
--
Matthieu Baerts | R&D Engineer
matthieu.baerts(a)tessares.net
Tessares SA | Hybrid Access Solutions
www.tessares.net
1 Avenue Jean Monnet, 1348 Louvain-la-Neuve, Belgium
689
days inactive
698
days old
7 comments
3 participants
participants (3)
-
Mat Martineau -
Matthieu Baerts -
Peter Krystad