[PATCH 5.10.y] mptcp: fix sometimes-uninitialized warning

Matthieu Baerts (NGI0) posted 1 patch 1 week, 4 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/multipath-tcp/mptcp_net-next tags/patchew/20240930162345.3938790-2-matttbe@kernel.org
net/mptcp/subflow.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH 5.10.y] mptcp: fix sometimes-uninitialized warning
Posted by Matthieu Baerts (NGI0) 1 week, 4 days ago
Nathan reported this issue:

  $ make -skj"$(nproc)" ARCH=x86_64 LLVM=1 LLVM_IAS=1 mrproper allmodconfig net/mptcp/subflow.o
  net/mptcp/subflow.c:877:6: warning: variable 'incr' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
    877 |         if (WARN_ON_ONCE(offset > skb->len))
        |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  include/asm-generic/bug.h:101:33: note: expanded from macro 'WARN_ON_ONCE'
    101 | #define WARN_ON_ONCE(condition) ({                              \
        |                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    102 |         int __ret_warn_on = !!(condition);                      \
        |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    103 |         if (unlikely(__ret_warn_on))                            \
        |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    104 |                 __WARN_FLAGS(BUGFLAG_ONCE |                     \
        |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    105 |                              BUGFLAG_TAINT(TAINT_WARN));        \
        |                              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    106 |         unlikely(__ret_warn_on);                                \
        |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    107 | })
        | ~~
  net/mptcp/subflow.c:893:6: note: uninitialized use occurs here
    893 |         if (incr)
        |             ^~~~
  net/mptcp/subflow.c:877:2: note: remove the 'if' if its condition is always false
    877 |         if (WARN_ON_ONCE(offset > skb->len))
        |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    878 |                 goto out;
        |                 ~~~~~~~~
  net/mptcp/subflow.c:874:18: note: initialize the variable 'incr' to silence this warning
    874 |         u32 offset, incr, avail_len;
        |                         ^
        |                          = 0
  1 warning generated.

As mentioned by Nathan, this issue is present because 5.10 does not
include commit ea4ca586b16f ("mptcp: refine MPTCP-level ack scheduling"),
which removed the use of 'incr' in the error path added by this change.
This other commit does not really look suitable for stable, hence this
dedicated patch for 5.10.

Fixes: e93fa44f0714 ("mptcp: fix duplicate data handling")
Reported-by: Nathan Chancellor <nathan@kernel.org>
Closes: https://lore.kernel.org/20240928175524.GA1713144@thelio-3990X
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/subflow.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 8a0ef50c307c..843c61ebd421 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -871,7 +871,7 @@ static void mptcp_subflow_discard_data(struct sock *ssk, struct sk_buff *skb,
 	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
 	bool fin = TCP_SKB_CB(skb)->tcp_flags & TCPHDR_FIN;
 	struct tcp_sock *tp = tcp_sk(ssk);
-	u32 offset, incr, avail_len;
+	u32 offset, incr = 0, avail_len;
 
 	offset = tp->copied_seq - TCP_SKB_CB(skb)->seq;
 	if (WARN_ON_ONCE(offset > skb->len))
-- 
2.45.2
Re: [PATCH 5.10.y] mptcp: fix sometimes-uninitialized warning
Posted by Greg KH 1 week, 3 days ago
On Mon, Sep 30, 2024 at 06:23:46PM +0200, Matthieu Baerts (NGI0) wrote:
> Nathan reported this issue:
> 
>   $ make -skj"$(nproc)" ARCH=x86_64 LLVM=1 LLVM_IAS=1 mrproper allmodconfig net/mptcp/subflow.o
>   net/mptcp/subflow.c:877:6: warning: variable 'incr' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
>     877 |         if (WARN_ON_ONCE(offset > skb->len))
>         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   include/asm-generic/bug.h:101:33: note: expanded from macro 'WARN_ON_ONCE'
>     101 | #define WARN_ON_ONCE(condition) ({                              \
>         |                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>     102 |         int __ret_warn_on = !!(condition);                      \
>         |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>     103 |         if (unlikely(__ret_warn_on))                            \
>         |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>     104 |                 __WARN_FLAGS(BUGFLAG_ONCE |                     \
>         |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>     105 |                              BUGFLAG_TAINT(TAINT_WARN));        \
>         |                              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>     106 |         unlikely(__ret_warn_on);                                \
>         |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>     107 | })
>         | ~~
>   net/mptcp/subflow.c:893:6: note: uninitialized use occurs here
>     893 |         if (incr)
>         |             ^~~~
>   net/mptcp/subflow.c:877:2: note: remove the 'if' if its condition is always false
>     877 |         if (WARN_ON_ONCE(offset > skb->len))
>         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>     878 |                 goto out;
>         |                 ~~~~~~~~
>   net/mptcp/subflow.c:874:18: note: initialize the variable 'incr' to silence this warning
>     874 |         u32 offset, incr, avail_len;
>         |                         ^
>         |                          = 0
>   1 warning generated.
> 
> As mentioned by Nathan, this issue is present because 5.10 does not
> include commit ea4ca586b16f ("mptcp: refine MPTCP-level ack scheduling"),
> which removed the use of 'incr' in the error path added by this change.
> This other commit does not really look suitable for stable, hence this
> dedicated patch for 5.10.
> 
> Fixes: e93fa44f0714 ("mptcp: fix duplicate data handling")
> Reported-by: Nathan Chancellor <nathan@kernel.org>
> Closes: https://lore.kernel.org/20240928175524.GA1713144@thelio-3990X
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
>  net/mptcp/subflow.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Now applied, thanks!

greg k-h
Re: [PATCH 5.10.y] mptcp: fix sometimes-uninitialized warning
Posted by MPTCP CI 1 week, 4 days ago
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/11110355350

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


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)
Patch "mptcp: fix sometimes-uninitialized warning" has been added to the 5.10-stable tree
Posted by gregkh@linuxfoundation.org 1 week, 3 days ago

This is a note to let you know that I've just added the patch titled

    mptcp: fix sometimes-uninitialized warning

to the 5.10-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     mptcp-fix-sometimes-uninitialized-warning.patch
and it can be found in the queue-5.10 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.


From matttbe@kernel.org  Tue Oct  1 10:14:56 2024
From: "Matthieu Baerts (NGI0)" <matttbe@kernel.org>
Date: Mon, 30 Sep 2024 18:23:46 +0200
Subject: mptcp: fix sometimes-uninitialized warning
To: stable@vger.kernel.org, gregkh@linuxfoundation.org
Cc: MPTCP Upstream <mptcp@lists.linux.dev>, "Matthieu Baerts (NGI0)" <matttbe@kernel.org>, Nathan Chancellor <nathan@kernel.org>
Message-ID: <20240930162345.3938790-2-matttbe@kernel.org>

From: "Matthieu Baerts (NGI0)" <matttbe@kernel.org>

Nathan reported this issue:

  $ make -skj"$(nproc)" ARCH=x86_64 LLVM=1 LLVM_IAS=1 mrproper allmodconfig net/mptcp/subflow.o
  net/mptcp/subflow.c:877:6: warning: variable 'incr' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
    877 |         if (WARN_ON_ONCE(offset > skb->len))
        |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  include/asm-generic/bug.h:101:33: note: expanded from macro 'WARN_ON_ONCE'
    101 | #define WARN_ON_ONCE(condition) ({                              \
        |                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    102 |         int __ret_warn_on = !!(condition);                      \
        |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    103 |         if (unlikely(__ret_warn_on))                            \
        |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    104 |                 __WARN_FLAGS(BUGFLAG_ONCE |                     \
        |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    105 |                              BUGFLAG_TAINT(TAINT_WARN));        \
        |                              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    106 |         unlikely(__ret_warn_on);                                \
        |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    107 | })
        | ~~
  net/mptcp/subflow.c:893:6: note: uninitialized use occurs here
    893 |         if (incr)
        |             ^~~~
  net/mptcp/subflow.c:877:2: note: remove the 'if' if its condition is always false
    877 |         if (WARN_ON_ONCE(offset > skb->len))
        |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    878 |                 goto out;
        |                 ~~~~~~~~
  net/mptcp/subflow.c:874:18: note: initialize the variable 'incr' to silence this warning
    874 |         u32 offset, incr, avail_len;
        |                         ^
        |                          = 0
  1 warning generated.

As mentioned by Nathan, this issue is present because 5.10 does not
include commit ea4ca586b16f ("mptcp: refine MPTCP-level ack scheduling"),
which removed the use of 'incr' in the error path added by this change.
This other commit does not really look suitable for stable, hence this
dedicated patch for 5.10.

Fixes: e93fa44f0714 ("mptcp: fix duplicate data handling")
Reported-by: Nathan Chancellor <nathan@kernel.org>
Closes: https://lore.kernel.org/20240928175524.GA1713144@thelio-3990X
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 net/mptcp/subflow.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -871,7 +871,7 @@ static void mptcp_subflow_discard_data(s
 	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
 	bool fin = TCP_SKB_CB(skb)->tcp_flags & TCPHDR_FIN;
 	struct tcp_sock *tp = tcp_sk(ssk);
-	u32 offset, incr, avail_len;
+	u32 offset, incr = 0, avail_len;
 
 	offset = tp->copied_seq - TCP_SKB_CB(skb)->seq;
 	if (WARN_ON_ONCE(offset > skb->len))


Patches currently in stable-queue which might be from matttbe@kernel.org are

queue-5.10/mptcp-export-lookup_anno_list_by_saddr.patch
queue-5.10/mptcp-validate-id-when-stopping-the-add_addr-retransmit-timer.patch
queue-5.10/mptcp-fix-sometimes-uninitialized-warning.patch
queue-5.10/mptcp-pm-fix-uaf-in-__timer_delete_sync.patch