mbox series

[mptcp-net,v3,0/2] mptcp: prevent MPC handshake on port-based signal endpoints

Message ID 20241008-mpc-hs-port-v3-0-cec1363f0353@kernel.org (mailing list archive)
Headers show
Series mptcp: prevent MPC handshake on port-based signal endpoints | expand

Message

Matthieu Baerts (NGI0) Oct. 8, 2024, 3:35 p.m. UTC
This is a new version for the series sent by Paolo, related to the
lockdep splat Syzbot reported a few weeks ago, and for which Cong Wang
sent a different attempt (Thanks both!).

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Changes in v3:
- patch 1: reduce comments to fit under 80 chars
- patch 2:
  - Add Fixes tag + comment about it
  - Add my Co-dev tag instead of RvB
  - Add a note about the new helpers in the commit message
  - move cappid declaration under capout
  - move 'cat $capout' to cond_stop_capture
  - move 'local port' declaration above to keep the reversed Xmas tree
  - use 'retl' var instead of 'ret' to avoid a clash with the one used
    in fail_test(), which would not have marked the whole selftest as
    failed in case of problem only with this test.
- Link to v2: https://lore.kernel.org/r/20241007-mpc-hs-port-v2-0-0c9e7827bd0f@kernel.org

Changes in v2:
- patch 1:
  - moving and renaming the MIB counters
  - move comments/functions on one line when possbile
  - rename subflow_reset_req -> _endp
  - use MPTCP_RST_EPROHIBIT reason
- patch 2:
  - fix shellcheck issues
  - capture: handle $capout
  - fix indentation issue
  - helper for the port
  - stop the capture after transfer like before
  - move new test to 'add addr ports' ones
  - move the validation part to a new helper
  - add missing 'check' names (subtitles)
  - skip if no counters
- Link to v1: https://lore.kernel.org/r/833cae5982ac5d5b3236845c6db4315e634f5705.1727974826.git.pabeni@redhat.com

---
Paolo Abeni (2):
      mptcp: prevent MPC handshake on port-based signal endpoints
      selftests: mptcp: join: add test-case for MPC attempt towards signl ep

 net/mptcp/mib.c                                 |   1 +
 net/mptcp/mib.h                                 |   1 +
 net/mptcp/pm_netlink.c                          |   1 +
 net/mptcp/protocol.h                            |   1 +
 net/mptcp/subflow.c                             |  11 +++
 tools/testing/selftests/net/mptcp/mptcp_join.sh | 117 +++++++++++++++++-------
 6 files changed, 101 insertions(+), 31 deletions(-)
---
base-commit: 2ce52e0f26d35f97f56615bb7854afa633f5d231
change-id: 20241007-mpc-hs-port-13a6c6c11b38

Best regards,

Comments

MPTCP CI Oct. 8, 2024, 4:45 p.m. UTC | #1
Hi Paolo,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal: Success! ✅
- KVM Validation: debug: Success! ✅
- KVM Validation: btf (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/11239121123

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/d11f11dec586
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=896784


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)
Mat Martineau Oct. 8, 2024, 11:40 p.m. UTC | #2
On Tue, 8 Oct 2024, Matthieu Baerts (NGI0) wrote:

> This is a new version for the series sent by Paolo, related to the
> lockdep splat Syzbot reported a few weeks ago, and for which Cong Wang
> sent a different attempt (Thanks both!).
>
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
> Changes in v3:
> - patch 1: reduce comments to fit under 80 chars
> - patch 2:
>  - Add Fixes tag + comment about it
>  - Add my Co-dev tag instead of RvB
>  - Add a note about the new helpers in the commit message
>  - move cappid declaration under capout
>  - move 'cat $capout' to cond_stop_capture
>  - move 'local port' declaration above to keep the reversed Xmas tree
>  - use 'retl' var instead of 'ret' to avoid a clash with the one used
>    in fail_test(), which would not have marked the whole selftest as
>    failed in case of problem only with this test.
> - Link to v2: https://lore.kernel.org/r/20241007-mpc-hs-port-v2-0-0c9e7827bd0f@kernel.org

Hi Matthieu -

v3 looks good to me, one minor change is to fix the "signl" typo in the 
subject line of patch 2 when applying.

Reviewed-by: Mat Martineau <martineau@kernel.org>

>
> Changes in v2:
> - patch 1:
>  - moving and renaming the MIB counters
>  - move comments/functions on one line when possbile
>  - rename subflow_reset_req -> _endp
>  - use MPTCP_RST_EPROHIBIT reason
> - patch 2:
>  - fix shellcheck issues
>  - capture: handle $capout
>  - fix indentation issue
>  - helper for the port
>  - stop the capture after transfer like before
>  - move new test to 'add addr ports' ones
>  - move the validation part to a new helper
>  - add missing 'check' names (subtitles)
>  - skip if no counters
> - Link to v1: https://lore.kernel.org/r/833cae5982ac5d5b3236845c6db4315e634f5705.1727974826.git.pabeni@redhat.com
>
> ---
> Paolo Abeni (2):
>      mptcp: prevent MPC handshake on port-based signal endpoints
>      selftests: mptcp: join: add test-case for MPC attempt towards signl ep
>
> net/mptcp/mib.c                                 |   1 +
> net/mptcp/mib.h                                 |   1 +
> net/mptcp/pm_netlink.c                          |   1 +
> net/mptcp/protocol.h                            |   1 +
> net/mptcp/subflow.c                             |  11 +++
> tools/testing/selftests/net/mptcp/mptcp_join.sh | 117 +++++++++++++++++-------
> 6 files changed, 101 insertions(+), 31 deletions(-)
> ---
> base-commit: 2ce52e0f26d35f97f56615bb7854afa633f5d231
> change-id: 20241007-mpc-hs-port-13a6c6c11b38
>
> Best regards,
> -- 
> Matthieu Baerts (NGI0) <matttbe@kernel.org>
>
>
>
Matthieu Baerts (NGI0) Oct. 9, 2024, 3:30 p.m. UTC | #3
Hi Mat,

On 09/10/2024 01:40, Mat Martineau wrote:
> On Tue, 8 Oct 2024, Matthieu Baerts (NGI0) wrote:
> 
>> This is a new version for the series sent by Paolo, related to the
>> lockdep splat Syzbot reported a few weeks ago, and for which Cong Wang
>> sent a different attempt (Thanks both!).
>>
>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>> ---
>> Changes in v3:
>> - patch 1: reduce comments to fit under 80 chars
>> - patch 2:
>>  - Add Fixes tag + comment about it
>>  - Add my Co-dev tag instead of RvB
>>  - Add a note about the new helpers in the commit message
>>  - move cappid declaration under capout
>>  - move 'cat $capout' to cond_stop_capture
>>  - move 'local port' declaration above to keep the reversed Xmas tree
>>  - use 'retl' var instead of 'ret' to avoid a clash with the one used
>>    in fail_test(), which would not have marked the whole selftest as
>>    failed in case of problem only with this test.
>> - Link to v2: https://lore.kernel.org/r/20241007-mpc-hs-port-
>> v2-0-0c9e7827bd0f@kernel.org
> 
> Hi Matthieu -
> 
> v3 looks good to me, one minor change is to fix the "signl" typo in the
> subject line of patch 2 when applying.
Thank you for the review!

Good catch! I then slightly changed the subject that was a bit too long
with 'signal endp' to ...

  test for prohibited MPC to port-based endp

... similar to the comment in mib.h.


Now in our tree (fixes for -net):

New patches for t/upstream-net and t/upstream:
- 5fc467636d0e: mptcp: prevent MPC handshake on port-based signal endpoints
- 4e5550c94f5d: selftests: mptcp: join: test for prohibited MPC to
port-based endp
- Results: 3bedd77fcc0a..6b1ad544c6f8 (export-net)
- 776972a2f26f: conflict in
t/mptcp-annotate-data-races-around-subflow-fully_established
- Results: 1d4b8739f94e..f1fcd89f7da0 (export)

Tests are now in progress:

- export-net:
https://github.com/multipath-tcp/mptcp_net-next/commit/2b9543b8dffd45921a4b440ccd7a03f4f42e3d6b/checks
- export:
https://github.com/multipath-tcp/mptcp_net-next/commit/08d19140c7fd6fdffd6866c648637fa6373e53fe/checks


Cheers,
Matt