[PATCH -next] mptcp: fix the incorrect judgment for msk->cb_flags

[PATCH -next] mptcp: fix the incorrect judgment for msk->cb_flags

[Xiang Yang]

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

Re: [PATCH -next] mptcp: fix the incorrect judgment for msk->cb_flags

[Matthieu Baerts]

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

Re: [PATCH -next] mptcp: fix the incorrect judgment for msk->cb_flags

[Jakub Kicinski]

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?

Re: [PATCH -next] mptcp: fix the incorrect judgment for msk->cb_flags

[Matthieu Baerts]

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

Re: [PATCH -next] mptcp: fix the incorrect judgment for msk->cb_flags

[Jakub Kicinski]

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!

Re: [PATCH -next] mptcp: fix the incorrect judgment for msk->cb_flags

[patchwork-bot+netdevbpf]

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