From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2F79DC83F2C for ; Tue, 5 Sep 2023 11:17:29 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 141EE94000D; Tue, 5 Sep 2023 07:17:29 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 0CAC18E001A; Tue, 5 Sep 2023 07:17:29 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id ED58B94000D; Tue, 5 Sep 2023 07:17:28 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id D9CAB8E001A for ; Tue, 5 Sep 2023 07:17:28 -0400 (EDT) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id B518CB3F8F for ; Tue, 5 Sep 2023 11:17:28 +0000 (UTC) X-FDA: 81202292976.07.9E24FE0 Received: from mail-io1-f49.google.com (mail-io1-f49.google.com [209.85.166.49]) by imf28.hostedemail.com (Postfix) with ESMTP id E9D50C0016 for ; Tue, 5 Sep 2023 11:17:26 +0000 (UTC) Authentication-Results: imf28.hostedemail.com; dkim=pass header.d=joelfernandes.org header.s=google header.b=UOffNxk8; dmarc=none; spf=pass (imf28.hostedemail.com: domain of joel@joelfernandes.org designates 209.85.166.49 as permitted sender) smtp.mailfrom=joel@joelfernandes.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1693912647; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=QFr3guuenGyb3Qn0aVjZt6j2hEqhzlyTzGXreRyxiz4=; b=V4v6EYYqeIsk/0NHZ0hRNji3wyFmQFDEZB6kS4w1xANFvgWxVNFsoo7zXgqWKKIZG5njO6 IUuHbXXqnNaZyBpI6AnXqYrIDPaBnQPZJQVi8Fr4X++aT8Cq13syzLqCrLt4Y+jyeuZW6S ztTpfijqykCgEVZgMVOcnvRgwGVFieg= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=pass header.d=joelfernandes.org header.s=google header.b=UOffNxk8; dmarc=none; spf=pass (imf28.hostedemail.com: domain of joel@joelfernandes.org designates 209.85.166.49 as permitted sender) smtp.mailfrom=joel@joelfernandes.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1693912647; a=rsa-sha256; cv=none; b=O/4RC3NDKO5051X3Nctdesb8KJKT6qYnDiRhR66FVvyx9dpiRYJgDLItUIFFxLAMkQXWIn XZyX5WRL9inYPWR83+a+L0CLjXnLN3jCohQ7xRKFnXM5ny2cxuXHREJe4oZLYAbC4GitzA 8zmvTIOZ29T51jPafp+XeQI7eE4T39U= Received: by mail-io1-f49.google.com with SMTP id ca18e2360f4ac-7926de0478eso65516039f.0 for ; Tue, 05 Sep 2023 04:17:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=joelfernandes.org; s=google; t=1693912646; x=1694517446; darn=kvack.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=QFr3guuenGyb3Qn0aVjZt6j2hEqhzlyTzGXreRyxiz4=; b=UOffNxk8vZe/m+kQX5hMet3gxdQIQrzW2xCMgmDteM6sRJWkquMp0xPxxlTw5yfMny lhadptlHIrzlPdLzxPXCsli17nI1W1mHB7mAC+E8RSs18FAkvQg1FQCA/K4zhMS7zARW fMaK6TqfcMpuR6aIGj5VessTCrE3d3HYyc0Yo= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1693912646; x=1694517446; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=QFr3guuenGyb3Qn0aVjZt6j2hEqhzlyTzGXreRyxiz4=; b=jUM0DsrEzBPM4WI4Wdg6UIjtjqaDbxioy8JTkb/HOTPx+jW9RgdwkgVpW3Wr41EG8x gjCex0kt06UI/oKE88vK79u/OrtS0n3/c8loKucelZ2lkhdDW484jxh88R5LvTuSFAeo vVh3p2UWOliqT0GOYy3XfWnR/anFGwgi6t+TRv1AV5ZDSm3QOcz/neD+EaHwXyMZrWTU DWvlgDGIT1t9IfLTm4aWKqx3ga23EHpX0oGjZhUHffSsKYXeYcjcwbt2DjDC2yxx1lOs XkTuS+hOIOhRzMq6Nt33KFsroexdbmwEhs3p9SDe+xoGEavbwKwXpRkZEct3x3pnM6dW 9Eiw== X-Gm-Message-State: AOJu0YzZlPfTrwp3ZWIYNytyhv9fbGqWBsVW9JHYR1CkFYYOEbwQXRaP VWGwKVi27CrZYlrnB2/uZf81VA== X-Google-Smtp-Source: AGHT+IH6ATLWvEQISqMYhYAvtisc3efnqHLoJKTqFihvNieOXuIdabwt3JbtmE5N7gFtoTpoNI0Ylw== X-Received: by 2002:a05:6602:19c5:b0:794:eb37:b0da with SMTP id ba5-20020a05660219c500b00794eb37b0damr10714307iob.2.1693912645977; Tue, 05 Sep 2023 04:17:25 -0700 (PDT) Received: from localhost (156.190.123.34.bc.googleusercontent.com. [34.123.190.156]) by smtp.gmail.com with ESMTPSA id n7-20020a6b7207000000b0077e3acd5ea1sm4221764ioc.53.2023.09.05.04.17.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 05 Sep 2023 04:17:25 -0700 (PDT) Date: Tue, 5 Sep 2023 11:17:25 +0000 From: Joel Fernandes To: Catalin Marinas Cc: Christoph Paasch , Andrew Morton , linux-mm@kvack.org, MPTCP Upstream , rcu@vger.kernel.org Subject: Re: kmemleak handling of kfree_rcu Message-ID: <20230905111725.GA3737639@google.com> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspam-User: X-Stat-Signature: egjbdka9q8m4pgpth6wpt8r79uzhjgsj X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: E9D50C0016 X-HE-Tag: 1693912646-957051 X-HE-Meta: U2FsdGVkX19vjyBxGTuAI7BRDQzo7EGOj7fXgLwlv+zRMlpFUOlwKmRaU0I9p5V3a0li3xV6K4AMGW3jvFo5uIWmfIRZGewmKj+IvFmo9MGIvxMAnH46fS9iio84L4XQMe/6kwe/Clsm6sxtatj9z5RNsEVSrPBXfiy0ktAeRlRK/YgQe5zhjbWQsbLldSSLLL1q7ZBca3nfju50WUcUc2e2a7sMBrozgBAWnUcpi73epZjVcGu0uuGNSlMr1voZHRGIMcoLRAYN7W5xa+cTIKB3x4ein+rIofTiXwNB88M6F8eonqFHc9VJVVFUnE8vN8XzYuWBzGaxdLubSo31YujNRzY8ZkLmZs3ip2ahvkH8GhIlCMQpZAr1fKI6xOl1BE13e81yIuIGhkaznSXSgrKAeceaCevoCde8N4bJZGP0ltysmkjHCyo67oBoXYjUNCkgExdlG1XA7vDebjIREnACxrZTYqynTLgbQf+pSOQCnRHJCltuYQS/yoxKYBm7jpQ/I+LpoMNppLoMiYaWGEIeXIsmEMxyHpSyM4MJfGZ2VJwG1IbTHvryTvMTq/6RMPofFQNGnDAa5XIdN2vKXoNJn9mMZTB5RHh10yt+YlJeV2Au7XKoH0bn3bbGQ7zfU/ozCNXPYFlesvX4oekL5YZK83jw4EDho0w/ZPCsYKCjN6ocxGjBsXS1pXhsP2yU59eOoypiZSrtHISPF3LVHD5sGJIdHmmqRblJr/e+trRqxB4fLP7lbuXyWOxh4E8Xo1Dv+y7S3lJyPDVvFntDIjsFuGKvymB8UxZfeOBhvOjMq70UeKkho+Baum0lY/DOGA6UyyeM4gc4QaZZBlZ6KuR3swLLsEJaaaHYC44ewJJW+Sse1sAtNCWK0zdsU7I4wbvqacL+7hgCkqqBU9im1KuSWUGO5pTz8FP2JOPSvV0FtMT772zty9iftpfDZgb1KO72oa4PXYB/pvPLu1A zFqZuxx/ G1CORpxdIjqRyp8QAAjwPKrbfrE6+Y9LgpD7/1CC5lO22lhXDjldE3lVq46eIXdrl72xHwzrc2cieQXPgmTAVCmqEgh//6wk/KchIU+zk9vz0x4J5l/sSalFGpnTPzY/xiQB1zvax+uP47XMt5mRk9WagNjbOksxONfU5wnoL1h5l1LzFNbiLgxthkyKrdIeKh0aNzP78QhIlrUlOvNI61V3HUltrjmfM1ip3SGesYimx2O5oWEMxbAE7iyJGCS7Fo1zuQKyyGV6BAkQRenInWq5tj5JnRdt6NOHjjjWTizlgcBC5UYo4Oqr+sNjZJLDdkzuN0IZdVfak/9hWQbkv4u3BJ40wW9ZrexOxhjCRCC1nsPzbplrajtCl7LVJdfYF4ljwWNj0svCEXmyLClwoTEbjMHJSl9BQmxVp X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Mon, Sep 04, 2023 at 10:22:56PM +0100, Catalin Marinas wrote: > Hi Christoph, > > Please try not to send html, many servers block such emails. > > Also adding the RCU list. > > On Wed, Aug 30, 2023 at 09:37:23AM -0700, Christoph Paasch wrote: > > for the MPTCP upstream project, we are running syzkaller [1] continuously to > > qualify our kernel changes. > > > > We found one issue with kmemleak and its handling of kfree_rcu. > > > > Specifically, it looks like kmemleak falsely reports memory-leaks when the > > object is being freed by kfree_rcu after a certain grace-period. > > > > For example, https://github.com/multipath-tcp/mptcp_net-next/issues/398# > > issuecomment-1584819482 shows how the syzkaller program reliably produces a > > kmemleak report, although the object is not leaked (we confirmed that by simply > > increasing MSECS_MIN_AGE to something larger than the grace-period). > > Not sure which RCU variant you are using but most likely the false > positives are caused by the original reference to the object being lost > and the pointer added to a new location that kmemleak does not track > (e.g. bnode->records[] in the tree-based variant). > > A quick attempt (untested, not even compiled): > > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c I am not sure if that works. Correct me if I'm wrong but the issue is that the allocated pointer being RCU-freed is no longer reachable by kmemleak (it is scanned but not reachable), however it might still be reachable via an RCU reader. In such a situation, it is a false-positive. The correct fix then should probably be to mark the object as kmemleak_not_leak() until a grace period elapses. This will cause the object to not be reported but still be scanned until eventually the lower layers will remove the object from kmemleak-tracking after the grace period. Per the docs also, that API is used to prevent false-positives. Instead what the below diff appears to do is to mark the bnode cache as a kmemleak object itself, which smells a bit wrong. The bnode is not an allocated object in the traditional sense, it is simple an internal data structure. That may not solve the issue as the false-positive unreachable object is still unreachable. Or did I misunderstand how kmemleak works? (Quite possible). thanks, - Joel > index 1449cb69a0e0..681a1eb7560a 100644 > --- a/kernel/rcu/tree.c > +++ b/kernel/rcu/tree.c > @@ -53,6 +53,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -2934,6 +2935,7 @@ drain_page_cache(struct kfree_rcu_cpu *krcp) > > llist_for_each_safe(pos, n, page_list) { > free_page((unsigned long)pos); > + kmemleak_free(pos); > freed++; > } > > @@ -2962,8 +2964,16 @@ kvfree_rcu_bulk(struct kfree_rcu_cpu *krcp, > rcu_state.name, bnode->records[i], 0); > > vfree(bnode->records[i]); > + /* avoid false negatives */ > + kmemleak_erase(&bnode->records[i]); > } > } > + /* > + * Avoid kmemleak false negatives when such pointers are later > + * re-allocated. > + */ > + for (i = 0; i < bnode->nr_records; i++) > + kmemleak_erase(&bnode->records[i]); > rcu_lock_release(&rcu_callback_map); > } > > @@ -2972,8 +2982,10 @@ kvfree_rcu_bulk(struct kfree_rcu_cpu *krcp, > bnode = NULL; > raw_spin_unlock_irqrestore(&krcp->lock, flags); > > - if (bnode) > + if (bnode) { > free_page((unsigned long) bnode); > + kmemleak_free(bnode); > + } > > cond_resched_tasks_rcu_qs(); > } > @@ -3241,6 +3253,12 @@ static void fill_page_cache_func(struct work_struct *work) > free_page((unsigned long) bnode); > break; > } > + > + /* > + * Scan the bnode->records[] array until the objects are > + * actually freed. > + */ > + kmemleak_alloc(bnode, PAGE_SIZE, 0, GFP_KERNEL); > } > > atomic_set(&krcp->work_in_progress, 0); > @@ -3308,6 +3326,11 @@ add_ptr_to_bulk_krc_lock(struct kfree_rcu_cpu **krcp, > // scenarios. > bnode = (struct kvfree_rcu_bulk_data *) > __get_free_page(GFP_KERNEL | __GFP_NORETRY | __GFP_NOMEMALLOC | __GFP_NOWARN); > + /* > + * Scan the bnode->records[] array until the objects are > + * actually freed. > + */ > + kmemleak_alloc(bnode, PAGE_SIZE, 0, GFP_KERNEL); > raw_spin_lock_irqsave(&(*krcp)->lock, *flags); > } > > > -- > Catalin