From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
alan@lxorguk.ukuu.org.uk,
Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>,
Paul Gortmaker <paul.gortmaker@windriver.com>,
Benjamin Gaignard <benjamin.gaignard@stericsson.com>,
Andrew Morton <akpm@linux-foundation.org>,
Linus Torvalds <torvalds@linux-foundation.org>
Subject: [ 04/34] genalloc: stop crashing the system when destroying a pool
Date: Mon, 29 Oct 2012 13:07:35 -0700 [thread overview]
Message-ID: <20121029200421.066146599@linuxfoundation.org> (raw)
In-Reply-To: <20121029200420.550338074@linuxfoundation.org>
3.0-stable review patch. If anyone has any objections, please let me know.
------------------
From: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
commit eedce141cd2dad8d0cefc5468ef41898949a7031 upstream.
The genalloc code uses the bitmap API from include/linux/bitmap.h and
lib/bitmap.c, which is based on long values. Both bitmap_set from
lib/bitmap.c and bitmap_set_ll, which is the lockless version from
genalloc.c, use BITMAP_LAST_WORD_MASK to set the first bits in a long in
the bitmap.
That one uses (1 << bits) - 1, 0b111, if you are setting the first three
bits. This means that the API counts from the least significant bits
(LSB from now on) to the MSB. The LSB in the first long is bit 0, then.
The same works for the lookup functions.
The genalloc code uses longs for the bitmap, as it should. In
include/linux/genalloc.h, struct gen_pool_chunk has unsigned long
bits[0] as its last member. When allocating the struct, genalloc should
reserve enough space for the bitmap. This should be a proper number of
longs that can fit the amount of bits in the bitmap.
However, genalloc allocates an integer number of bytes that fit the
amount of bits, but may not be an integer amount of longs. 9 bytes, for
example, could be allocated for 70 bits.
This is a problem in itself if the Least Significat Bit in a long is in
the byte with the largest address, which happens in Big Endian machines.
This means genalloc is not allocating the byte in which it will try to
set or check for a bit.
This may end up in memory corruption, where genalloc will try to set the
bits it has not allocated. In fact, genalloc may not set these bits
because it may find them already set, because they were not zeroed since
they were not allocated. And that's what causes a BUG when
gen_pool_destroy is called and check for any set bits.
What really happens is that genalloc uses kmalloc_node with __GFP_ZERO
on gen_pool_add_virt. With SLAB and SLUB, this means the whole slab
will be cleared, not only the requested bytes. Since struct
gen_pool_chunk has a size that is a multiple of 8, and slab sizes are
multiples of 8, we get lucky and allocate and clear the right amount of
bytes.
Hower, this is not the case with SLOB or with older code that did memset
after allocating instead of using __GFP_ZERO.
So, a simple module as this (running 3.6.0), will cause a crash when
rmmod'ed.
[root@phantom-lp2 foo]# cat foo.c
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/init.h>
#include <linux/genalloc.h>
MODULE_LICENSE("GPL");
MODULE_VERSION("0.1");
static struct gen_pool *foo_pool;
static __init int foo_init(void)
{
int ret;
foo_pool = gen_pool_create(10, -1);
if (!foo_pool)
return -ENOMEM;
ret = gen_pool_add(foo_pool, 0xa0000000, 32 << 10, -1);
if (ret) {
gen_pool_destroy(foo_pool);
return ret;
}
return 0;
}
static __exit void foo_exit(void)
{
gen_pool_destroy(foo_pool);
}
module_init(foo_init);
module_exit(foo_exit);
[root@phantom-lp2 foo]# zcat /proc/config.gz | grep SLOB
CONFIG_SLOB=y
[root@phantom-lp2 foo]# insmod ./foo.ko
[root@phantom-lp2 foo]# rmmod foo
------------[ cut here ]------------
kernel BUG at lib/genalloc.c:243!
cpu 0x4: Vector: 700 (Program Check) at [c0000000bb0e7960]
pc: c0000000003cb50c: .gen_pool_destroy+0xac/0x110
lr: c0000000003cb4fc: .gen_pool_destroy+0x9c/0x110
sp: c0000000bb0e7be0
msr: 8000000000029032
current = 0xc0000000bb0e0000
paca = 0xc000000006d30e00 softe: 0 irq_happened: 0x01
pid = 13044, comm = rmmod
kernel BUG at lib/genalloc.c:243!
[c0000000bb0e7ca0] d000000004b00020 .foo_exit+0x20/0x38 [foo]
[c0000000bb0e7d20] c0000000000dff98 .SyS_delete_module+0x1a8/0x290
[c0000000bb0e7e30] c0000000000097d4 syscall_exit+0x0/0x94
--- Exception: c00 (System Call) at 000000800753d1a0
SP (fffd0b0e640) is in userspace
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
Cc: Benjamin Gaignard <benjamin.gaignard@stericsson.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
lib/genalloc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/lib/genalloc.c
+++ b/lib/genalloc.c
@@ -57,7 +57,7 @@ int gen_pool_add_virt(struct gen_pool *p
struct gen_pool_chunk *chunk;
int nbits = size >> pool->min_alloc_order;
int nbytes = sizeof(struct gen_pool_chunk) +
- (nbits + BITS_PER_BYTE - 1) / BITS_PER_BYTE;
+ BITS_TO_LONGS(nbits) * sizeof(long);
chunk = kmalloc_node(nbytes, GFP_KERNEL | __GFP_ZERO, nid);
if (unlikely(chunk == NULL))
next prev parent reply other threads:[~2012-10-29 20:07 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-10-29 20:07 [ 00/34] 3.0.50-stable review Greg Kroah-Hartman
2012-10-29 20:07 ` [ 01/34] gen_init_cpio: avoid stack overflow when expanding Greg Kroah-Hartman
2012-10-29 20:07 ` [ 02/34] fs/compat_ioctl.c: VIDEO_SET_SPU_PALETTE missing error check Greg Kroah-Hartman
2012-10-29 20:07 ` [ 03/34] drivers/rtc/rtc-imxdi.c: add missing spin lock initialization Greg Kroah-Hartman
2012-10-29 20:07 ` Greg Kroah-Hartman [this message]
2012-10-29 20:07 ` [ 05/34] ARM: 7559/1: smp: switch away from the idmap before updating init_mm.mm_count Greg Kroah-Hartman
2012-10-29 20:07 ` [ 06/34] SUNRPC: Get rid of the xs_error_report socket callback Greg Kroah-Hartman
2012-10-29 20:07 ` [ 07/34] SUNRPC: Clear the connect flag when socket state is TCP_CLOSE_WAIT Greg Kroah-Hartman
2012-10-29 20:07 ` [ 08/34] Revert "SUNRPC: Ensure we close the socket on EPIPE errors too..." Greg Kroah-Hartman
2012-10-29 20:07 ` [ 09/34] SUNRPC: Prevent races in xs_abort_connection() Greg Kroah-Hartman
2012-10-29 20:07 ` [ 10/34] sysfs: sysfs_pathname/sysfs_add_one: Use strlcat() instead of strcat() Greg Kroah-Hartman
2012-10-29 20:07 ` [ 11/34] ehci: fix Lucid nohandoff pci quirk to be more generic with BIOS versions Greg Kroah-Hartman
2012-10-29 20:07 ` [ 12/34] ehci: Add yet-another Lucid nohandoff pci quirk Greg Kroah-Hartman
2012-10-29 20:07 ` [ 13/34] usb-storage: add unusual_devs entry for Casio EX-N1 digital camera Greg Kroah-Hartman
2012-10-29 20:07 ` [ 14/34] usb hub: send clear_tt_buffer_complete events when canceling TT clear work Greg Kroah-Hartman
2012-10-29 20:07 ` [ 15/34] USB: whiteheat: fix memory leak in error path Greg Kroah-Hartman
2012-10-29 20:07 ` [ 16/34] USB: opticon: fix DMA from stack Greg Kroah-Hartman
2012-10-29 20:07 ` [ 17/34] USB: opticon: fix memory leak in error path Greg Kroah-Hartman
2012-10-29 20:07 ` [ 18/34] USB: serial: Fix memory leak in sierra_release() Greg Kroah-Hartman
2012-10-29 20:07 ` [ 19/34] USB: sierra: fix memory leak in attach error path Greg Kroah-Hartman
2012-10-29 20:07 ` [ 20/34] USB: sierra: fix memory leak in probe " Greg Kroah-Hartman
2012-10-29 20:07 ` [ 21/34] USB: mos7840: fix urb leak at release Greg Kroah-Hartman
2012-10-29 20:07 ` [ 22/34] USB: mos7840: fix port-device leak in error path Greg Kroah-Hartman
2012-10-29 20:07 ` [ 23/34] USB: mos7840: remove NULL-urb submission Greg Kroah-Hartman
2012-10-29 20:07 ` [ 24/34] USB: mos7840: remove invalid disconnect handling Greg Kroah-Hartman
2012-10-29 20:07 ` [ 25/34] xhci: Fix potential NULL ptr deref in command cancellation Greg Kroah-Hartman
2012-10-29 20:07 ` [ 26/34] vhost: fix mergeable bufs on BE hosts Greg Kroah-Hartman
2012-10-29 20:07 ` [ 27/34] ARM: at91/i2c: change id to let i2c-gpio work Greg Kroah-Hartman
2012-10-29 20:07 ` [ 28/34] mac80211: check if key has TKIP type before updating IV Greg Kroah-Hartman
2012-10-29 20:08 ` [ 29/34] bcma: fix unregistration of cores Greg Kroah-Hartman
2012-10-29 20:08 ` [ 30/34] cpufreq / powernow-k8: Remove usage of smp_processor_id() in preemptible code Greg Kroah-Hartman
2012-10-29 20:08 ` [ 31/34] x86, mm: Find_early_table_space based on ranges that are actually being mapped Greg Kroah-Hartman
2012-10-29 20:08 ` [ 32/34] x86, mm: Undo incorrect revert in arch/x86/mm/init.c Greg Kroah-Hartman
2012-10-29 20:08 ` [ 33/34] staging: comedi: amplc_pc236: fix invalid register access during detach Greg Kroah-Hartman
2012-10-29 20:08 ` [ 34/34] drm/i915: no lvds quirk for Zotac ZDBOX SD ID12/ID13 Greg Kroah-Hartman
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=20121029200421.066146599@linuxfoundation.org \
--to=gregkh@linuxfoundation.org \
--cc=akpm@linux-foundation.org \
--cc=alan@lxorguk.ukuu.org.uk \
--cc=benjamin.gaignard@stericsson.com \
--cc=cascardo@linux.vnet.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=paul.gortmaker@windriver.com \
--cc=stable@vger.kernel.org \
--cc=torvalds@linux-foundation.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).