[PATCH 1/1] mm/percpu.c: append alignment sanity checkup to avoid memory leakage

[PATCH 1/1] mm/percpu.c: append alignment sanity checkup to avoid memory leakage

[zijun_hu]

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

[PATCH 1/1] mm/percpu.c: append alignment sanity checkup to avoid memory leakage

[zijun_hu]

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>

Re: [PATCH 1/1] mm/percpu.c: append alignment sanity checkup to avoid memory leakage

[Tejun Heo]

Hello,

I updated the patch description and code style a bit and applied to
percpu/for-4.10.

Thanks.

------ 8< ------
>From 3ca45a46f8af8c4a92dd8a08eac57787242d5021 Mon Sep 17 00:00:00 2001
From: zijun_hu <zijun_hu@htc.com>
Date: Fri, 14 Oct 2016 15:12:54 +0800
Subject: [PATCH] percpu: ensure the requested alignment is power of two

The percpu allocator expectedly assumes that the requested alignment
is power of two but hasn't been veryfing the input.  If the specified
alignment isn't power of two, the allocator can malfunction.  Add the
sanity check.

The following is detailed analysis of the effects of alignments which
aren't power of two.

 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 an odd alignment doesn't work.
 Let's 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.

tj: Code style and patch description updates.

Signed-off-by: zijun_hu <zijun_hu@htc.com>
Suggested-by: Tejun Heo <tj@kernel.org>
Signed-off-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 bc6ed52..0dc0b21 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 2557143..99d8abd 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;
-- 
2.7.4

Re: [PATCH 1/1] mm/percpu.c: append alignment sanity checkup to avoid memory leakage

[Tejun Heo]

Hello,

I updated the patch description and code style a bit and applied to
percpu/for-4.10.

Thanks.

------ 8< ------