Annotate data-races around `subflow->fully_established` #516

matttbe · 2024-08-29T14:07:48Z

This has been covered for the msk->fully_established field,, but it looks like it is not the case for the subflow one:

msk:

net/mptcp/pm_netlink.c:                if (!READ_ONCE(msk->fully_established) ||
net/mptcp/protocol.c:        WRITE_ONCE(msk->fully_established, false);
net/mptcp/protocol.c:        WRITE_ONCE(msk->fully_established, false);
net/mptcp/protocol.h:        bool                fully_established;
net/mptcp/protocol.h:               READ_ONCE(mptcp_sk(sk)->fully_established);
net/mptcp/subflow.c:        WRITE_ONCE(msk->fully_established, true);

subflow:

net/mptcp/diag.c:        if (sf->fully_established)
net/mptcp/options.c:        if (subflow->fully_established || snd_data_fin_enable ||
net/mptcp/options.c:        if (likely(subflow->fully_established)) {
net/mptcp/protocol.c:        if (mptcp_subflow_ctx(ssk)->fully_established)
net/mptcp/protocol.h:                fully_established : 1,            /* path validated */
net/mptcp/protocol.h:        if (subflow->request_join && !subflow->fully_established)
net/mptcp/subflow.c:        subflow->fully_established = 1;
net/mptcp/subflow.c:                return !subflow->fully_established;
net/mptcp/subflow.c:                new_ctx->fully_established = 1;

The text was updated successfully, but these errors were encountered:

We introduce the same handling for potential data races with the 'fully_established' flag in subflow as previously done for msk->fully_established. Additionally, we make a crucial change: convert the subflow's 'fully_established' from 'bit_field' to 'bool' type. This is necessary because methods for avoiding data races don't work well with 'bit_field'. Specifically, the 'READ_ONCE' needs to know the size of the variable being accessed, which is not supported in 'bit_field'. Also, 'test_bit' expect the address of 'bit_field'. This change was prompted by compilation errors we encountered, as detailed below. ''' READ_ONCE: 477 | (sizeof(t) == sizeof(char) || \ | ^ sizeof(t) == sizeof(short)|| \ ././include/linux/compiler_types.h:490:23: note: in definition of macro ‘__compiletime_assert’ 490 | if (!(condition))\ | ^~~~~~~~~ ././include/linux/compiler_types.h:510:9: note: in expansion of macro ‘_compiletime_assert’ 510 | _compiletime_assert(condition, msg, \ __compiletime_assert_, __COUNTER__) | ^~~~~~~~~~~~~~~~~~~ ./include/asm-generic/rwonce.h:36:9: note: in expansion of macro ‘compiletime_assert’ 36 | compiletime_assert(__native_word(t) || \ sizeof(t) == sizeof(long long), \ | ^~~~~~~~~~~~~~~~~~ ./include/asm-generic/rwonce.h:36:28: note: in expansion of macro ‘__native_word’ 36 | compiletime_assert(__native_word(t) || \ | ^~~~~~~~~~~~~ sizeof(t) == sizeof(long long), \ ./include/asm-generic/rwonce.h:49:9: note: in expansion of macro ‘compiletime_assert_rwonce_type’ 49 | compiletime_assert_rwonce_type(x); \ | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ net/mptcp/protocol.h:781:39: note: in expansion of macro ‘READ_ONCE’ 781 | if (subflow->request_join && \ | !READ_ONCE(subflow->fully_established)) | ^~~~~~~~~ test_bit: error: cannot take address of bit-field 'fully_established' 50 | if (test_bit(1, &sf->fully_established)) | ''' Closes: multipath-tcp/mptcp_net-next#516 Signed-off-by: YANGANG <[email protected]>

We introduce the same handling for potential data races with the 'fully_established' flag in subflow as previously done for msk->fully_established. Additionally, we make a crucial change: convert the subflow's 'fully_established' from 'bit_field' to 'bool' type. This is necessary because methods for avoiding data races don't work well with 'bit_field'. Specifically, the 'READ_ONCE' needs to know the size of the variable being accessed, which is not supported in 'bit_field'. Also, 'test_bit' expect the address of 'bit_field'. This change was prompted by compilation errors we encountered, as detailed below. ''' READ_ONCE: 477 | (sizeof(t) == sizeof(char) || \ | ^ sizeof(t) == sizeof(short)|| \ ././include/linux/compiler_types.h:490:23: note: in definition of macro ‘__compiletime_assert’ 490 | if (!(condition))\ | ^~~~~~~~~ ././include/linux/compiler_types.h:510:9: note: in expansion of macro ‘_compiletime_assert’ 510 | _compiletime_assert(condition, msg, \ __compiletime_assert_, __COUNTER__) | ^~~~~~~~~~~~~~~~~~~ ./include/asm-generic/rwonce.h:36:9: note: in expansion of macro ‘compiletime_assert’ 36 | compiletime_assert(__native_word(t) || \ sizeof(t) == sizeof(long long), \ | ^~~~~~~~~~~~~~~~~~ ./include/asm-generic/rwonce.h:36:28: note: in expansion of macro ‘__native_word’ 36 | compiletime_assert(__native_word(t) || \ | ^~~~~~~~~~~~~ sizeof(t) == sizeof(long long), \ ./include/asm-generic/rwonce.h:49:9: note: in expansion of macro ‘compiletime_assert_rwonce_type’ 49 | compiletime_assert_rwonce_type(x); \ | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ net/mptcp/protocol.h:781:39: note: in expansion of macro ‘READ_ONCE’ 781 | if (subflow->request_join && \ | !READ_ONCE(subflow->fully_established)) | ^~~~~~~~~ test_bit: error: cannot take address of bit-field 'fully_established' 50 | if (test_bit(1, &sf->fully_established)) | ''' Closes: #516 Signed-off-by: YANGANG <[email protected]> Message-Id: <[email protected]>

We introduce the same handling for potential data races with the 'fully_established' flag in subflow as previously done for msk->fully_established. Additionally, we make a crucial change: convert the subflow's 'fully_established' from 'bit_field' to 'bool' type. This is necessary because methods for avoiding data races don't work well with 'bit_field'. Specifically, the 'READ_ONCE' needs to know the size of the variable being accessed, which is not supported in 'bit_field'. Also, 'test_bit' expect the address of 'bit_field'. Closes: #516 Signed-off-by: YANGANG <[email protected]> Reviewed-by: Matthieu Baerts (NGI0) <[email protected]>

We introduce the same handling for potential data races with the 'fully_established' flag in subflow as previously done for msk->fully_established. Additionally, we make a crucial change: convert the subflow's 'fully_established' from 'bit_field' to 'bool' type. This is necessary because methods for avoiding data races don't work well with 'bit_field'. Specifically, the 'READ_ONCE' needs to know the size of the variable being accessed, which is not supported in 'bit_field'. Also, 'test_bit' expect the address of 'bit_field'. Closes: #516 Signed-off-by: Gang Yan <[email protected]> Reviewed-by: Matthieu Baerts (NGI0) <[email protected]>

We introduce the same handling for potential data races with the 'fully_established' flag in subflow as previously done for msk->fully_established. Additionally, we make a crucial change: convert the subflow's 'fully_established' from 'bit_field' to 'bool' type. This is necessary because methods for avoiding data races don't work well with 'bit_field'. Specifically, the 'READ_ONCE' needs to know the size of the variable being accessed, which is not supported in 'bit_field'. Also, 'test_bit' expect the address of 'bit_field'. Closes: multipath-tcp/mptcp_net-next#516 Signed-off-by: Gang Yan <[email protected]> Reviewed-by: Matthieu Baerts (NGI0) <[email protected]> Signed-off-by: Matthieu Baerts (NGI0) <[email protected]> Signed-off-by: NipaLocal <nipa@local>

We introduce the same handling for potential data races with the 'fully_established' flag in subflow as previously done for msk->fully_established. Additionally, we make a crucial change: convert the subflow's 'fully_established' from 'bit_field' to 'bool' type. This is necessary because methods for avoiding data races don't work well with 'bit_field'. Specifically, the 'READ_ONCE' needs to know the size of the variable being accessed, which is not supported in 'bit_field'. Also, 'test_bit' expect the address of 'bit_field'. Closes: #516 Signed-off-by: Gang Yan <[email protected]> Reviewed-by: Matthieu Baerts (NGI0) <[email protected]>

We introduce the same handling for potential data races with the 'fully_established' flag in subflow as previously done for msk->fully_established. Additionally, we make a crucial change: convert the subflow's 'fully_established' from 'bit_field' to 'bool' type. This is necessary because methods for avoiding data races don't work well with 'bit_field'. Specifically, the 'READ_ONCE' needs to know the size of the variable being accessed, which is not supported in 'bit_field'. Also, 'test_bit' expect the address of 'bit_field'. Closes: multipath-tcp/mptcp_net-next#516 Signed-off-by: Gang Yan <[email protected]> Reviewed-by: Matthieu Baerts (NGI0) <[email protected]> Signed-off-by: Matthieu Baerts (NGI0) <[email protected]> Signed-off-by: NipaLocal <nipa@local>

We introduce the same handling for potential data races with the 'fully_established' flag in subflow as previously done for msk->fully_established. Additionally, we make a crucial change: convert the subflow's 'fully_established' from 'bit_field' to 'bool' type. This is necessary because methods for avoiding data races don't work well with 'bit_field'. Specifically, the 'READ_ONCE' needs to know the size of the variable being accessed, which is not supported in 'bit_field'. Also, 'test_bit' expect the address of 'bit_field'. Closes: #516 Signed-off-by: Gang Yan <[email protected]> Reviewed-by: Matthieu Baerts (NGI0) <[email protected]>

We introduce the same handling for potential data races with the 'fully_established' flag in subflow as previously done for msk->fully_established. Additionally, we make a crucial change: convert the subflow's 'fully_established' from 'bit_field' to 'bool' type. This is necessary because methods for avoiding data races don't work well with 'bit_field'. Specifically, the 'READ_ONCE' needs to know the size of the variable being accessed, which is not supported in 'bit_field'. Also, 'test_bit' expect the address of 'bit_field'. Closes: multipath-tcp/mptcp_net-next#516 Signed-off-by: Gang Yan <[email protected]> Reviewed-by: Matthieu Baerts (NGI0) <[email protected]> Signed-off-by: Matthieu Baerts (NGI0) <[email protected]> Signed-off-by: NipaLocal <nipa@local>

We introduce the same handling for potential data races with the 'fully_established' flag in subflow as previously done for msk->fully_established. Additionally, we make a crucial change: convert the subflow's 'fully_established' from 'bit_field' to 'bool' type. This is necessary because methods for avoiding data races don't work well with 'bit_field'. Specifically, the 'READ_ONCE' needs to know the size of the variable being accessed, which is not supported in 'bit_field'. Also, 'test_bit' expect the address of 'bit_field'. Closes: multipath-tcp/mptcp_net-next#516 Signed-off-by: Gang Yan <[email protected]> Reviewed-by: Matthieu Baerts (NGI0) <[email protected]> Signed-off-by: Matthieu Baerts (NGI0) <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>

[ Upstream commit 581c8cb ] We introduce the same handling for potential data races with the 'fully_established' flag in subflow as previously done for msk->fully_established. Additionally, we make a crucial change: convert the subflow's 'fully_established' from 'bit_field' to 'bool' type. This is necessary because methods for avoiding data races don't work well with 'bit_field'. Specifically, the 'READ_ONCE' needs to know the size of the variable being accessed, which is not supported in 'bit_field'. Also, 'test_bit' expect the address of 'bit_field'. Closes: multipath-tcp/mptcp_net-next#516 Signed-off-by: Gang Yan <[email protected]> Reviewed-by: Matthieu Baerts (NGI0) <[email protected]> Signed-off-by: Matthieu Baerts (NGI0) <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Jakub Kicinski <[email protected]> Signed-off-by: Sasha Levin <[email protected]>

matttbe added the bug label Aug 29, 2024

matttbe closed this as completed in 5704783 Sep 9, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Annotate data-races around `subflow->fully_established` #516

Annotate data-races around `subflow->fully_established` #516

matttbe commented Aug 29, 2024 •

edited

Loading

Annotate data-races around subflow->fully_established #516

Annotate data-races around subflow->fully_established #516

Comments

matttbe commented Aug 29, 2024 • edited Loading

Annotate data-races around `subflow->fully_established` #516

Annotate data-races around `subflow->fully_established` #516

matttbe commented Aug 29, 2024 •

edited

Loading