Re: [TEST] TCP MD5 vs kmemleak

Hi Paul,

Sorry for the delayed answer, I got a respiratory infection, so was in
bed last week,

On Thu, 20 Jun 2024 at 18:02, Paul E. McKenney [off-list ref] wrote:

quoted

On Wed, Jun 19, 2024 at 01:33:36AM +0100, Dmitry Safonov wrote:

[..]

quoted

quoted

I'm sorry guys, that was me being inadequate.

I know that feeling!

Thanks :)

[adding/quoting back the context of the thread that I cut previously]

quoted

On Tue, Jun 18, 2024 at 10:02:10AM -0700, Jakub Kicinski wrote:

quoted

On Tue, 18 Jun 2024 09:42:35 -0700 Paul E. McKenney wrote:

quoted

quoted

FTR, with mptcp self-tests we hit a few kmemleak false positive on RCU
freed pointers, that where addressed by to this patch:

commit 5f98fd034ca6fd1ab8c91a3488968a0e9caaabf6
Author: Catalin Marinas [off-list ref]
Date:   Sat Sep 30 17:46:56 2023 +0000

    rcu: kmemleak: Ignore kmemleak false positives when RCU-freeing objects

I'm wondering if this is hitting something similar? Possibly due to
lazy RCU callbacks invoked after MSECS_MIN_AGE???

Dmitry mentioned this commit, too, but we use the same config for MPTCP
tests, and while we repro TCP AO failures quite frequently, mptcp
doesn't seem to have failed once.

quoted

Fun!  ;-)

This commit handles memory passed to kfree_rcu() and friends, but
not memory passed to call_rcu() and friends.  Of course, call_rcu()
does not necessarily know the full extent of the memory passed to it,
for example, if passed a linked list, call_rcu() will know only about
the head of that list.

There are similar challenges with synchronize_rcu() and friends.

To be clear I think Dmitry was suspecting kfree_rcu(), he mentioned
call_rcu() as something he was expecting to have a similar issue but
it in fact appeared immune.

On Thu, 20 Jun 2024 at 18:02, Paul E. McKenney [off-list ref] wrote:

quoted

quoted

That's a real issue, rather than a false-positive:
https://lore.kernel.org/netdev/20240619-tcp-ao-required-leak-v1-1-6408f3c94247@gmail.com/ (local)

So we need call_rcu() to mark memory flowing through it?  If so, we
need help from callers of call_rcu() in the case where more than one
object is being freed.

Not sure, I think one way to avoid explicitly marking pointers for
call_rcu() or even avoiding the patch above would be a hackery like
this:

diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index d5b6fba44fc9..7a5eb55a155c 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c

@@ -1587,6 +1587,7 @@ static void kmemleak_cond_resched(struct

kmemleak_object *object)
 static void kmemleak_scan(void)
 {
        struct kmemleak_object *object;
+       unsigned long gp_start_state;
        struct zone *zone;
        int __maybe_unused i;
        int new_leaks = 0;

@@ -1630,6 +1631,7 @@ static void kmemleak_scan(void)
                        kmemleak_cond_resched(object);
        }
        rcu_read_unlock();
+       gp_start_state = get_state_synchronize_rcu();

 #ifdef CONFIG_SMP
        /* per-cpu sections scanning */

@@ -1690,6 +1692,13 @@ static void kmemleak_scan(void)
         */
        scan_gray_list();

+       /*
+        * Wait for the greylist objects potentially migrating from
+        * RCU callbacks or maybe getting freed.
+        */
+       cond_synchronize_rcu(gp_start_state);
+       rcu_barrier();