* [PATCH -next] mptcp: fix the incorrect judgment for msk->cb_flags
@ 2023-08-03 7:24 Xiang Yang
2023-08-03 16:32 ` Matthieu Baerts
2023-08-04 22:40 ` patchwork-bot+netdevbpf
0 siblings, 2 replies; 6+ messages in thread
From: Xiang Yang @ 2023-08-03 7:24 UTC (permalink / raw
To: matthieu.baerts, martineau, davem, edumazet, kuba, pabeni
Cc: netdev, mptcp, xiangyang3
Coccicheck reports the error below:
net/mptcp/protocol.c:3330:15-28: ERROR: test of a variable/field address
Since the address of msk->cb_flags is used in __test_and_clear_bit, the
address should not be NULL. The judgment for if (unlikely(msk->cb_flags))
will always be true, we should check the real value of msk->cb_flags here.
Fixes: 65a569b03ca8 ("mptcp: optimize release_cb for the common case")
Signed-off-by: Xiang Yang <xiangyang3@huawei.com>
---
net/mptcp/protocol.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 65ee949a8a44..fae31dab49c9 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -3327,7 +3327,7 @@ static void mptcp_release_cb(struct sock *sk)
if (__test_and_clear_bit(MPTCP_CLEAN_UNA, &msk->cb_flags))
__mptcp_clean_una_wakeup(sk);
- if (unlikely(&msk->cb_flags)) {
+ if (unlikely(msk->cb_flags)) {
/* be sure to set the current sk state before tacking actions
* depending on sk_state, that is processing MPTCP_ERROR_REPORT
*/
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH -next] mptcp: fix the incorrect judgment for msk->cb_flags
2023-08-03 7:24 [PATCH -next] mptcp: fix the incorrect judgment for msk->cb_flags Xiang Yang
@ 2023-08-03 16:32 ` Matthieu Baerts
2023-08-03 18:04 ` Jakub Kicinski
2023-08-04 22:40 ` patchwork-bot+netdevbpf
1 sibling, 1 reply; 6+ messages in thread
From: Matthieu Baerts @ 2023-08-03 16:32 UTC (permalink / raw
To: Xiang Yang, martineau, davem, edumazet, kuba, pabeni; +Cc: netdev, mptcp
Hi Xiang Yang
On 03/08/2023 09:24, Xiang Yang wrote:
> Coccicheck reports the error below:
> net/mptcp/protocol.c:3330:15-28: ERROR: test of a variable/field address
>
> Since the address of msk->cb_flags is used in __test_and_clear_bit, the
> address should not be NULL. The judgment for if (unlikely(msk->cb_flags))
> will always be true, we should check the real value of msk->cb_flags here.
>
> Fixes: 65a569b03ca8 ("mptcp: optimize release_cb for the common case")
> Signed-off-by: Xiang Yang <xiangyang3@huawei.com>
This Coccicheck report was useful, the optimisation in place was not
working. But there was no impact apart from testing more conditions
where there were no reasons to.
The fix is then good to me but it should land in -net, not in net-next.
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
I don't know if it is needed to have a re-send just to change the subject.
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH -next] mptcp: fix the incorrect judgment for msk->cb_flags
2023-08-03 16:32 ` Matthieu Baerts
@ 2023-08-03 18:04 ` Jakub Kicinski
2023-08-03 20:18 ` Matthieu Baerts
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Kicinski @ 2023-08-03 18:04 UTC (permalink / raw
To: Matthieu Baerts
Cc: Xiang Yang, martineau, davem, edumazet, pabeni, netdev, mptcp
On Thu, 3 Aug 2023 18:32:15 +0200 Matthieu Baerts wrote:
> This Coccicheck report was useful, the optimisation in place was not
> working. But there was no impact apart from testing more conditions
> where there were no reasons to.
>
> The fix is then good to me but it should land in -net, not in net-next.
>
> Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
>
> I don't know if it is needed to have a re-send just to change the subject.
Looks trivial enough to apply without a repost, but are you sure
you don't want to take it into your tree? Run the selftests and all?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH -next] mptcp: fix the incorrect judgment for msk->cb_flags
2023-08-03 18:04 ` Jakub Kicinski
@ 2023-08-03 20:18 ` Matthieu Baerts
2023-08-03 20:55 ` Jakub Kicinski
0 siblings, 1 reply; 6+ messages in thread
From: Matthieu Baerts @ 2023-08-03 20:18 UTC (permalink / raw
To: Jakub Kicinski
Cc: Xiang Yang, martineau, davem, edumazet, pabeni, netdev, mptcp
Hi Jakub,
3 Aug 2023 20:04:26 Jakub Kicinski <kuba@kernel.org>:
> On Thu, 3 Aug 2023 18:32:15 +0200 Matthieu Baerts wrote:
>> This Coccicheck report was useful, the optimisation in place was not
>> working. But there was no impact apart from testing more conditions
>> where there were no reasons to.
>>
>> The fix is then good to me but it should land in -net, not in net-next.
>>
>> Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
>>
>> I don't know if it is needed to have a re-send just to change the subject.
>
> Looks trivial enough to apply without a repost, but are you sure
> you don't want to take it into your tree? Run the selftests and all?
Thank you for asking that! All patches sent to our mailing list are automatically tested but the report is only sent to our mailing list not to annoy too many people. This patch passed all tests we have:
https://lore.kernel.org/mptcp/20230803072438.1847500-1-xiangyang3@huawei.com/T/
I already applied it on our side. For non-trivial fixes or features, we usually prefer to keep them a bit only applied on our side for longer tests and to have syzkaller stressing them. But here, because this patch looks trivial enough, it seems fine to me to have it applied in -net directly.
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH -next] mptcp: fix the incorrect judgment for msk->cb_flags
2023-08-03 20:18 ` Matthieu Baerts
@ 2023-08-03 20:55 ` Jakub Kicinski
0 siblings, 0 replies; 6+ messages in thread
From: Jakub Kicinski @ 2023-08-03 20:55 UTC (permalink / raw
To: Matthieu Baerts
Cc: Xiang Yang, martineau, davem, edumazet, pabeni, netdev, mptcp
On Thu, 3 Aug 2023 22:18:18 +0200 (GMT+02:00) Matthieu Baerts wrote:
> Thank you for asking that! All patches sent to our mailing list are
> automatically tested but the report is only sent to our mailing list
> not to annoy too many people. This patch passed all tests we have:
>
> https://lore.kernel.org/mptcp/20230803072438.1847500-1-xiangyang3@huawei.com/T/
>
> I already applied it on our side. For non-trivial fixes or features,
> we usually prefer to keep them a bit only applied on our side for
> longer tests and to have syzkaller stressing them. But here, because
> this patch looks trivial enough, it seems fine to me to have it
> applied in -net directly.
GTK! I'll apply it in the evening, thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH -next] mptcp: fix the incorrect judgment for msk->cb_flags
2023-08-03 7:24 [PATCH -next] mptcp: fix the incorrect judgment for msk->cb_flags Xiang Yang
2023-08-03 16:32 ` Matthieu Baerts
@ 2023-08-04 22:40 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-08-04 22:40 UTC (permalink / raw
To: Xiang Yang
Cc: matthieu.baerts, martineau, davem, edumazet, kuba, pabeni, netdev,
mptcp
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Thu, 3 Aug 2023 07:24:38 +0000 you wrote:
> Coccicheck reports the error below:
> net/mptcp/protocol.c:3330:15-28: ERROR: test of a variable/field address
>
> Since the address of msk->cb_flags is used in __test_and_clear_bit, the
> address should not be NULL. The judgment for if (unlikely(msk->cb_flags))
> will always be true, we should check the real value of msk->cb_flags here.
>
> [...]
Here is the summary with links:
- [-next] mptcp: fix the incorrect judgment for msk->cb_flags
https://git.kernel.org/netdev/net/c/17ebf8a4c38b
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-08-04 22:40 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-03 7:24 [PATCH -next] mptcp: fix the incorrect judgment for msk->cb_flags Xiang Yang
2023-08-03 16:32 ` Matthieu Baerts
2023-08-03 18:04 ` Jakub Kicinski
2023-08-03 20:18 ` Matthieu Baerts
2023-08-03 20:55 ` Jakub Kicinski
2023-08-04 22:40 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).