mbox series

[mptcp-net,0/2] mptcp: "fix suspicious RCU usage" warnings

Message ID 20241016-mptcp-sched-find-rcu-v1-0-5e9af4fbce11@kernel.org (mailing list archive)
Headers show
Series mptcp: "fix suspicious RCU usage" warnings | expand

Message

Matthieu Baerts Oct. 16, 2024, 7:05 p.m. UTC
Enabling PROVE_RCU_LIST (and RCU_EXPERT) shows list_for_each_entry_rcu()
from mptcp_sched_find() not being used with RCU read lock held.

The first patch is a fix for -net. The other one is for a commit that is
only in our tree.

Link: https://lore.kernel.org/20241016011144.3058445-1-kuba@kernel.org
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Matthieu Baerts (NGI0) (2):
      mptcp: init: protect sched with rcu_read_lock
      Squash to "bpf: Add bpf_mptcp_sched_ops"

 net/mptcp/bpf.c      | 10 +++++++---
 net/mptcp/protocol.c |  2 ++
 2 files changed, 9 insertions(+), 3 deletions(-)
---
base-commit: baccd7675477b1db387aa71b48c3312b5fb67a5d
change-id: 20241016-mptcp-sched-find-rcu-649ce3399334

Best regards,

Comments

MPTCP CI Oct. 16, 2024, 8:11 p.m. UTC | #1
Hi Matthieu,

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/11372377607

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


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)
Matthieu Baerts Oct. 17, 2024, 9:06 a.m. UTC | #2
Hi Geliang,

On 16/10/2024 21:05, Matthieu Baerts (NGI0) wrote:
> Enabling PROVE_RCU_LIST (and RCU_EXPERT) shows list_for_each_entry_rcu()
> from mptcp_sched_find() not being used with RCU read lock held.
> 
> The first patch is a fix for -net. The other one is for a commit that is
> only in our tree.
> 
> Link: https://lore.kernel.org/20241016011144.3058445-1-kuba@kernel.org
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
> Matthieu Baerts (NGI0) (2):
>       mptcp: init: protect sched with rcu_read_lock
>       Squash to "bpf: Add bpf_mptcp_sched_ops"

Thank you for the review! I just applied these two patches in our tree:

New patches for t/upstream-net and t/upstream:
- e80dfc53fa3d: mptcp: init: protect sched with rcu_read_lock
- Results: 32fda8b8e0f1..4367da1fbf63 (export-net)
- 8e293a58ad78: "squashed" patch 2/2 in "bpf: Add bpf_mptcp_sched_ops"
- Results: baccd7675477..0004ac084daf (export)

Tests are now in progress:

- export-net:
https://github.com/multipath-tcp/mptcp_net-next/commit/6b56229c81f9ee84435f808186d11e9bcb7ad6c3/checks
- export:
https://github.com/multipath-tcp/mptcp_net-next/commit/e8bb62da034e057e6d778b8d7c8c0c9c5cd5d5ab/checks

Cheers,
Matt
Paolo Abeni Oct. 17, 2024, 9:38 a.m. UTC | #3
Hi,

On 10/16/24 21:05, Matthieu Baerts (NGI0) wrote:
> Enabling PROVE_RCU_LIST (and RCU_EXPERT) shows list_for_each_entry_rcu()
> from mptcp_sched_find() not being used with RCU read lock held.
> 
> The first patch is a fix for -net. The other one is for a commit that is
> only in our tree.
> 
> Link: https://lore.kernel.org/20241016011144.3058445-1-kuba@kernel.org
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
> Matthieu Baerts (NGI0) (2):
>        mptcp: init: protect sched with rcu_read_lock
>        Squash to "bpf: Add bpf_mptcp_sched_ops"
> 
>   net/mptcp/bpf.c      | 10 +++++++---
>   net/mptcp/protocol.c |  2 ++
>   2 files changed, 9 insertions(+), 3 deletions(-)
> ---
> base-commit: baccd7675477b1db387aa71b48c3312b5fb67a5d
> change-id: 20241016-mptcp-sched-find-rcu-649ce3399334
> 
> Best regards,

Only slightly related, but mptcp_get_available_schedulers() currently 
acquires both the rcu and the sched_list lock. I think the latter is not 
needed and should be dropped.

Cheers,

Paolo
Matthieu Baerts Oct. 17, 2024, 9:47 a.m. UTC | #4
Hi Paolo,

On 17/10/2024 11:38, Paolo Abeni wrote:
> On 10/16/24 21:05, Matthieu Baerts (NGI0) wrote:
>> Enabling PROVE_RCU_LIST (and RCU_EXPERT) shows list_for_each_entry_rcu()
>> from mptcp_sched_find() not being used with RCU read lock held.
>>
>> The first patch is a fix for -net. The other one is for a commit that is
>> only in our tree.
>>
>> Link: https://lore.kernel.org/20241016011144.3058445-1-kuba@kernel.org
>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>> ---
>> Matthieu Baerts (NGI0) (2):
>>        mptcp: init: protect sched with rcu_read_lock
>>        Squash to "bpf: Add bpf_mptcp_sched_ops"
>>
>>   net/mptcp/bpf.c      | 10 +++++++---
>>   net/mptcp/protocol.c |  2 ++
>>   2 files changed, 9 insertions(+), 3 deletions(-)
>> ---
>> base-commit: baccd7675477b1db387aa71b48c3312b5fb67a5d
>> change-id: 20241016-mptcp-sched-find-rcu-649ce3399334
>>
>> Best regards,
> 
> Only slightly related, but mptcp_get_available_schedulers() currently
> acquires both the rcu and the sched_list lock. I think the latter is not
> needed and should be dropped.

Good point, the list is not modified.
I can send a patch for that.

(sorry, I should not have applied these patches that quickly, I was busy
adding 'debug' support for the BPF tests in our CI, and I didn't think
it was too quick)

Cheers,
Matt