All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vlastimil Babka <vbabka@suse.cz>
To: Andrew Morton <akpm@linux-foundation.org>,
	Christian Borntraeger <borntraeger@de.ibm.com>
Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	Vlastimil Babka <vbabka@suse.cz>,
	stable@vger.kernel.org,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	Boris Ostrovsky <boris.ostrovsky@oracle.com>,
	David Vrabel <david.vrabel@citrix.com>,
	Juergen Gross <jgross@suse.com>,
	"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Subject: [PATCH] mm, frontswap: make sure allocated frontswap map is assigned
Date: Wed, 26 Oct 2016 15:42:20 +0200	[thread overview]
Message-ID: <20161026134220.2566-1-vbabka@suse.cz> (raw)
In-Reply-To: <633c9485-d150-03ac-d0d3-827ad24c514d@de.ibm.com>

Christian Borntraeger reports:

with commit 8ea1d2a1985a7ae096e ("mm, frontswap: convert frontswap_enabled to
static key") kmemleak complains about a memory leak in swapon

unreferenced object 0x3e09ba56000 (size 32112640):
  comm "swapon", pid 7852, jiffies 4294968787 (age 1490.770s)
  hex dump (first 32 bytes):
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<00000000003a2504>] __vmalloc_node_range+0x194/0x2d8
    [<00000000003a2918>] vzalloc+0x58/0x68
    [<00000000003b0af0>] SyS_swapon+0xd60/0x12f8
    [<0000000000a3dc2e>] system_call+0xd6/0x270
    [<ffffffffffffffff>] 0xffffffffffffffff

Turns out kmemleak is right. We now allocate the frontswap map depending on the
kernel config (and no longer on the enablement)

swapfile.c:
[...]
      if (IS_ENABLED(CONFIG_FRONTSWAP))
                frontswap_map = vzalloc(BITS_TO_LONGS(maxpages) * sizeof(long));

but later on this is passed along
--> enable_swap_info(p, prio, swap_map, cluster_info, frontswap_map);

and ignored if frontswap is disabled
--> frontswap_init(p->type, frontswap_map);
static inline void frontswap_init(unsigned type, unsigned long *map)
{
        if (frontswap_enabled())
                __frontswap_init(type, map);
}

Thing is, that frontswap map is never freed.

===

The leakage is relatively not that bad, because swapon is an infrequent and
privileged operation. However, if the first frontswap backend is registered
after a swap type has been already enabled, it will WARN_ON in
frontswap_register_ops() and frontswap will not be available for the swap type.

Fix this by making sure the map is assigned by frontswap_init() as long as
CONFIG_FRONTSWAP is enabled.

Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
Fixes: 8ea1d2a1985a ("mm, frontswap: convert frontswap_enabled to static key")
Cc: stable@vger.kernel.org
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: David Vrabel <david.vrabel@citrix.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
 include/linux/frontswap.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/linux/frontswap.h b/include/linux/frontswap.h
index c46d2aa16d81..1d18af034554 100644
--- a/include/linux/frontswap.h
+++ b/include/linux/frontswap.h
@@ -106,8 +106,9 @@ static inline void frontswap_invalidate_area(unsigned type)
 
 static inline void frontswap_init(unsigned type, unsigned long *map)
 {
-	if (frontswap_enabled())
-		__frontswap_init(type, map);
+#ifdef CONFIG_FRONTSWAP
+	__frontswap_init(type, map);
+#endif
 }
 
 #endif /* _LINUX_FRONTSWAP_H */
-- 
2.10.1

WARNING: multiple messages have this Message-ID (diff)
From: Vlastimil Babka <vbabka@suse.cz>
To: Andrew Morton <akpm@linux-foundation.org>,
	Christian Borntraeger <borntraeger@de.ibm.com>
Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	Vlastimil Babka <vbabka@suse.cz>,
	stable@vger.kernel.org,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	Boris Ostrovsky <boris.ostrovsky@oracle.com>,
	David Vrabel <david.vrabel@citrix.com>,
	Juergen Gross <jgross@suse.com>,
	"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Subject: [PATCH] mm, frontswap: make sure allocated frontswap map is assigned
Date: Wed, 26 Oct 2016 15:42:20 +0200	[thread overview]
Message-ID: <20161026134220.2566-1-vbabka@suse.cz> (raw)
In-Reply-To: <633c9485-d150-03ac-d0d3-827ad24c514d@de.ibm.com>

Christian Borntraeger reports:

with commit 8ea1d2a1985a7ae096e ("mm, frontswap: convert frontswap_enabled to
static key") kmemleak complains about a memory leak in swapon

unreferenced object 0x3e09ba56000 (size 32112640):
  comm "swapon", pid 7852, jiffies 4294968787 (age 1490.770s)
  hex dump (first 32 bytes):
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<00000000003a2504>] __vmalloc_node_range+0x194/0x2d8
    [<00000000003a2918>] vzalloc+0x58/0x68
    [<00000000003b0af0>] SyS_swapon+0xd60/0x12f8
    [<0000000000a3dc2e>] system_call+0xd6/0x270
    [<ffffffffffffffff>] 0xffffffffffffffff

Turns out kmemleak is right. We now allocate the frontswap map depending on the
kernel config (and no longer on the enablement)

swapfile.c:
[...]
      if (IS_ENABLED(CONFIG_FRONTSWAP))
                frontswap_map = vzalloc(BITS_TO_LONGS(maxpages) * sizeof(long));

but later on this is passed along
--> enable_swap_info(p, prio, swap_map, cluster_info, frontswap_map);

and ignored if frontswap is disabled
--> frontswap_init(p->type, frontswap_map);
static inline void frontswap_init(unsigned type, unsigned long *map)
{
        if (frontswap_enabled())
                __frontswap_init(type, map);
}

Thing is, that frontswap map is never freed.

===

The leakage is relatively not that bad, because swapon is an infrequent and
privileged operation. However, if the first frontswap backend is registered
after a swap type has been already enabled, it will WARN_ON in
frontswap_register_ops() and frontswap will not be available for the swap type.

Fix this by making sure the map is assigned by frontswap_init() as long as
CONFIG_FRONTSWAP is enabled.

Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
Fixes: 8ea1d2a1985a ("mm, frontswap: convert frontswap_enabled to static key")
Cc: stable@vger.kernel.org
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: David Vrabel <david.vrabel@citrix.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
 include/linux/frontswap.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/linux/frontswap.h b/include/linux/frontswap.h
index c46d2aa16d81..1d18af034554 100644
--- a/include/linux/frontswap.h
+++ b/include/linux/frontswap.h
@@ -106,8 +106,9 @@ static inline void frontswap_invalidate_area(unsigned type)
 
 static inline void frontswap_init(unsigned type, unsigned long *map)
 {
-	if (frontswap_enabled())
-		__frontswap_init(type, map);
+#ifdef CONFIG_FRONTSWAP
+	__frontswap_init(type, map);
+#endif
 }
 
 #endif /* _LINUX_FRONTSWAP_H */
-- 
2.10.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2016-10-26 13:42 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-26 12:10 regression 4.8+ commit 8ea1d2a (mm, frontswap: convert frontswap_enabled to static key) cause memory leak on swapon Christian Borntraeger
2016-10-26 12:10 ` Christian Borntraeger
2016-10-26 13:42 ` Vlastimil Babka [this message]
2016-10-26 13:42   ` [PATCH] mm, frontswap: make sure allocated frontswap map is assigned Vlastimil Babka

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161026134220.2566-1-vbabka@suse.cz \
    --to=vbabka@suse.cz \
    --cc=akpm@linux-foundation.org \
    --cc=boris.ostrovsky@oracle.com \
    --cc=borntraeger@de.ibm.com \
    --cc=david.vrabel@citrix.com \
    --cc=jgross@suse.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=konrad.wilk@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.