Recently, during a debugging session using local MPTCP connections, I noticed MPJoinAckHMacFailure was strangely not zero on the server side.
The first patch fixes this issue -- present since v5.9 -- and the second one validates it in the selftests.
Signed-off-by: Matthieu Baerts (NGI0) matttbe@kernel.org --- Matthieu Baerts (NGI0) (2): mptcp: only inc MPJoinAckHMacFailure for HMAC failures selftests: mptcp: validate MPJoin HMacFailure counters
net/mptcp/subflow.c | 8 ++++++-- tools/testing/selftests/net/mptcp/mptcp_join.sh | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) --- base-commit: 61f96e684edd28ca40555ec49ea1555df31ba619 change-id: 20250407-net-mptcp-hmac-failure-mib-66f599305ff3
Best regards,
Recently, during a debugging session using local MPTCP connections, I noticed MPJoinAckHMacFailure was not zero on the server side. The counter was in fact incremented when the PM rejected new subflows, because the 'subflow' limit was reached.
The fix is easy, simply dissociating the two cases: only the HMAC validation check should increase MPTCP_MIB_JOINACKMAC counter.
Fixes: 4cf8b7e48a09 ("subflow: introduce and use mptcp_can_accept_new_subflow()") Cc: stable@vger.kernel.org Reviewed-by: Geliang Tang geliang@kernel.org Signed-off-by: Matthieu Baerts (NGI0) matttbe@kernel.org --- net/mptcp/subflow.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c index 409bd415ef1d190d5599658d01323ad8c8a9be93..24c2de1891bdf31dfe04ef2077113563aad0e666 100644 --- a/net/mptcp/subflow.c +++ b/net/mptcp/subflow.c @@ -899,13 +899,17 @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk, goto dispose_child; }
- if (!subflow_hmac_valid(req, &mp_opt) || - !mptcp_can_accept_new_subflow(subflow_req->msk)) { + if (!subflow_hmac_valid(req, &mp_opt)) { SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_JOINACKMAC); subflow_add_reset_reason(skb, MPTCP_RST_EPROHIBIT); goto dispose_child; }
+ if (!mptcp_can_accept_new_subflow(owner)) { + subflow_add_reset_reason(skb, MPTCP_RST_EPROHIBIT); + goto dispose_child; + } + /* move the msk reference ownership to the subflow */ subflow_req->msk = NULL; ctx->conn = (struct sock *)owner;
On Mon, Apr 07, 2025 at 08:26:32PM +0200, Matthieu Baerts (NGI0) wrote:
Recently, during a debugging session using local MPTCP connections, I noticed MPJoinAckHMacFailure was not zero on the server side. The counter was in fact incremented when the PM rejected new subflows, because the 'subflow' limit was reached.
The fix is easy, simply dissociating the two cases: only the HMAC validation check should increase MPTCP_MIB_JOINACKMAC counter.
Fixes: 4cf8b7e48a09 ("subflow: introduce and use mptcp_can_accept_new_subflow()") Cc: stable@vger.kernel.org Reviewed-by: Geliang Tang geliang@kernel.org Signed-off-by: Matthieu Baerts (NGI0) matttbe@kernel.org
Reviewed-by: Simon Horman horms@kernel.org
The parent commit fixes an issue around these counters where one of them -- MPJoinAckHMacFailure -- was wrongly incremented in some cases.
This makes sure the counter is always 0. It should be incremented only in case of corruption, or a wrong implementation, which should not be the case in these selftests.
Reviewed-by: Geliang Tang geliang@kernel.org Signed-off-by: Matthieu Baerts (NGI0) matttbe@kernel.org --- tools/testing/selftests/net/mptcp/mptcp_join.sh | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh index 13a3b68181ee14eb628a858e5738094c3c936b74..befa66f5a366bb738f8e6d6d84677f5c07488720 100755 --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh @@ -1441,6 +1441,15 @@ chk_join_nr() fi fi
+ count=$(mptcp_lib_get_counter ${ns2} "MPTcpExtMPJoinSynAckHMacFailure") + if [ -z "$count" ]; then + rc=${KSFT_SKIP} + elif [ "$count" != "0" ]; then + rc=${KSFT_FAIL} + print_check "synack HMAC" + fail_test "got $count JOIN[s] synack HMAC failure expected 0" + fi + count=$(mptcp_lib_get_counter ${ns1} "MPTcpExtMPJoinAckRx") if [ -z "$count" ]; then rc=${KSFT_SKIP} @@ -1450,6 +1459,15 @@ chk_join_nr() fail_test "got $count JOIN[s] ack rx expected $ack_nr" fi
+ count=$(mptcp_lib_get_counter ${ns1} "MPTcpExtMPJoinAckHMacFailure") + if [ -z "$count" ]; then + rc=${KSFT_SKIP} + elif [ "$count" != "0" ]; then + rc=${KSFT_FAIL} + print_check "ack HMAC" + fail_test "got $count JOIN[s] ack HMAC failure expected 0" + fi + print_results "join Rx" ${rc}
join_syn_tx="${join_syn_tx:-${syn_nr}}" \
On Mon, Apr 07, 2025 at 08:26:33PM +0200, Matthieu Baerts (NGI0) wrote:
The parent commit fixes an issue around these counters where one of them -- MPJoinAckHMacFailure -- was wrongly incremented in some cases.
This makes sure the counter is always 0. It should be incremented only in case of corruption, or a wrong implementation, which should not be the case in these selftests.
Reviewed-by: Geliang Tang geliang@kernel.org Signed-off-by: Matthieu Baerts (NGI0) matttbe@kernel.org
Reviewed-by: Simon Horman horms@kernel.org
Hello:
This series was applied to netdev/net.git (main) by Jakub Kicinski kuba@kernel.org:
On Mon, 07 Apr 2025 20:26:31 +0200 you wrote:
Recently, during a debugging session using local MPTCP connections, I noticed MPJoinAckHMacFailure was strangely not zero on the server side.
The first patch fixes this issue -- present since v5.9 -- and the second one validates it in the selftests.
Signed-off-by: Matthieu Baerts (NGI0) matttbe@kernel.org
[...]
Here is the summary with links: - [net,1/2] mptcp: only inc MPJoinAckHMacFailure for HMAC failures https://git.kernel.org/netdev/net/c/21c02e8272bc - [net,2/2] selftests: mptcp: validate MPJoin HMacFailure counters https://git.kernel.org/netdev/net/c/6767698cf9c1
You are awesome, thank you!
linux-kselftest-mirror@lists.linaro.org