All of lore.kernel.org
 help / color / mirror / Atom feed
From: zijun_hu <zijun_hu@zoho.com>
To: tj@kernel.org, Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	zijun_hu@htc.com, cl@linux.com
Subject: [PATCH 1/1] mm/percpu.c: append alignment sanity checkup to avoid memory leakage
Date: Fri, 14 Oct 2016 15:12:54 +0800	[thread overview]
Message-ID: <58008576.3060302@zoho.com> (raw)

From: zijun_hu <zijun_hu@htc.com>

the percpu allocator only works well currently when allocates a power of
2 aligned area, but there aren't any hints for alignment requirement, so
memory leakage maybe be caused if allocate other alignment areas

the alignment must be a even at least since the LSB of a chunk->map element
is used as free/in-use flag of a area; besides, the alignment must be a
power of 2 too since ALIGN() doesn't work well for other alignment always
but is adopted by pcpu_fit_in_area(). IOW, the current allocator only works
well for a power of 2 aligned area allocation.

see below opposite example for why a odd alignment doesn't work
lets assume area [16, 36) is free but its previous one is in-use, we want
to allocate a @size == 8 and @align == 7 area. the larger area [16, 36) is
split to three areas [16, 21), [21, 29), [29, 36) eventually. however, due
to the usage for a chunk->map element, the actual offset of the aim area
[21, 29) is 21 but is recorded in relevant element as 20; moreover the
residual tail free area [29, 36) is mistook as in-use and is lost silently

unlike macro roundup(), ALIGN(x, a) doesn't work if @a isn't a power of 2
for example, roundup(10, 6) == 12 but ALIGN(10, 6) == 10, and the latter
result isn't desired obviously.

fix it by appending sanity checkup for alignment requirement

Signed-off-by: zijun_hu <zijun_hu@htc.com>
Suggested-by: Tejun Heo <tj@kernel.org>
---
 include/linux/kernel.h | 1 +
 mm/percpu.c            | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index bc6ed52a39b9..0dc0b21bd164 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -45,6 +45,7 @@
 
 #define REPEAT_BYTE(x)	((~0ul / 0xff) * (x))
 
+/* @a is a power of 2 value */
 #define ALIGN(x, a)		__ALIGN_KERNEL((x), (a))
 #define __ALIGN_MASK(x, mask)	__ALIGN_KERNEL_MASK((x), (mask))
 #define PTR_ALIGN(p, a)		((typeof(p))ALIGN((unsigned long)(p), (a)))
diff --git a/mm/percpu.c b/mm/percpu.c
index 255714302394..10ba3f9a3826 100644
--- a/mm/percpu.c
+++ b/mm/percpu.c
@@ -886,7 +886,8 @@ static void __percpu *pcpu_alloc(size_t size, size_t align, bool reserved,
 
 	size = ALIGN(size, 2);
 
-	if (unlikely(!size || size > PCPU_MIN_UNIT_SIZE || align > PAGE_SIZE)) {
+	if (unlikely(!size || size > PCPU_MIN_UNIT_SIZE || align > PAGE_SIZE
+				|| !is_power_of_2(align))) {
 		WARN(true, "illegal size (%zu) or align (%zu) for percpu allocation\n",
 		     size, align);
 		return NULL;
-- 
1.9.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>

WARNING: multiple messages have this Message-ID (diff)
From: zijun_hu <zijun_hu@zoho.com>
To: tj@kernel.org, Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	zijun_hu@htc.com, cl@linux.com
Subject: [PATCH 1/1] mm/percpu.c: append alignment sanity checkup to avoid memory leakage
Date: Fri, 14 Oct 2016 15:12:54 +0800	[thread overview]
Message-ID: <58008576.3060302@zoho.com> (raw)

From: zijun_hu <zijun_hu@htc.com>

the percpu allocator only works well currently when allocates a power of
2 aligned area, but there aren't any hints for alignment requirement, so
memory leakage maybe be caused if allocate other alignment areas

the alignment must be a even at least since the LSB of a chunk->map element
is used as free/in-use flag of a area; besides, the alignment must be a
power of 2 too since ALIGN() doesn't work well for other alignment always
but is adopted by pcpu_fit_in_area(). IOW, the current allocator only works
well for a power of 2 aligned area allocation.

see below opposite example for why a odd alignment doesn't work
lets assume area [16, 36) is free but its previous one is in-use, we want
to allocate a @size == 8 and @align == 7 area. the larger area [16, 36) is
split to three areas [16, 21), [21, 29), [29, 36) eventually. however, due
to the usage for a chunk->map element, the actual offset of the aim area
[21, 29) is 21 but is recorded in relevant element as 20; moreover the
residual tail free area [29, 36) is mistook as in-use and is lost silently

unlike macro roundup(), ALIGN(x, a) doesn't work if @a isn't a power of 2
for example, roundup(10, 6) == 12 but ALIGN(10, 6) == 10, and the latter
result isn't desired obviously.

fix it by appending sanity checkup for alignment requirement

Signed-off-by: zijun_hu <zijun_hu@htc.com>
Suggested-by: Tejun Heo <tj@kernel.org>
---
 include/linux/kernel.h | 1 +
 mm/percpu.c            | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index bc6ed52a39b9..0dc0b21bd164 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -45,6 +45,7 @@
 
 #define REPEAT_BYTE(x)	((~0ul / 0xff) * (x))
 
+/* @a is a power of 2 value */
 #define ALIGN(x, a)		__ALIGN_KERNEL((x), (a))
 #define __ALIGN_MASK(x, mask)	__ALIGN_KERNEL_MASK((x), (mask))
 #define PTR_ALIGN(p, a)		((typeof(p))ALIGN((unsigned long)(p), (a)))
diff --git a/mm/percpu.c b/mm/percpu.c
index 255714302394..10ba3f9a3826 100644
--- a/mm/percpu.c
+++ b/mm/percpu.c
@@ -886,7 +886,8 @@ static void __percpu *pcpu_alloc(size_t size, size_t align, bool reserved,
 
 	size = ALIGN(size, 2);
 
-	if (unlikely(!size || size > PCPU_MIN_UNIT_SIZE || align > PAGE_SIZE)) {
+	if (unlikely(!size || size > PCPU_MIN_UNIT_SIZE || align > PAGE_SIZE
+				|| !is_power_of_2(align))) {
 		WARN(true, "illegal size (%zu) or align (%zu) for percpu allocation\n",
 		     size, align);
 		return NULL;
-- 
1.9.1

             reply	other threads:[~2016-10-14  7:13 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-14  7:12 zijun_hu [this message]
2016-10-14  7:12 ` [PATCH 1/1] mm/percpu.c: append alignment sanity checkup to avoid memory leakage zijun_hu
2016-10-19 18:01 ` Tejun Heo
2016-10-19 18:01   ` Tejun Heo

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=58008576.3060302@zoho.com \
    --to=zijun_hu@zoho.com \
    --cc=akpm@linux-foundation.org \
    --cc=cl@linux.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=tj@kernel.org \
    --cc=zijun_hu@htc.com \
    /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.