From ec9c96ef3cc0124cb94375b17faaa8cff5dfdf97 Mon Sep 17 00:00:00 2001 From: Kyle McMartin Date: Wed, 19 Aug 2009 21:17:08 -0400 Subject: dma-debug: Fix check_unmap null pointer dereference While it's debatable whether or not a NULL device argument to the DMA API functions is valid... since it certainly isn't valid on devices with an IOMMU... dma-debug really shouldn't be dereferencing null pointers either. Guard against that in err_printk and the driver_filter functions. A Fedora rawhide user was seeing this in one of the dvb drivers resulting in an oops on boot. [ A patch has been sent for testing to the driver, but I feel the dma debugging support should be fixed as well. (There's still a pile of legacy garbage in the kernel passing null pointers to dma_{alloc,free}_*. :( ] Signed-off-by: Kyle McMartin Cc: mchehab@infradead.org Cc: Joerg Roedel LKML-Reference: <20090820011708.GP25206@bombadil.infradead.org> Signed-off-by: Ingo Molnar --- lib/dma-debug.c | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) (limited to 'lib') diff --git a/lib/dma-debug.c b/lib/dma-debug.c index 65b0d99..58a9f9f 100644 --- a/lib/dma-debug.c +++ b/lib/dma-debug.c @@ -156,9 +156,13 @@ static bool driver_filter(struct device *dev) return true; /* driver filter on and initialized */ - if (current_driver && dev->driver == current_driver) + if (current_driver && dev && dev->driver == current_driver) return true; + /* driver filter on, but we can't filter on a NULL device... */ + if (!dev) + return false; + if (current_driver || !current_driver_name[0]) return false; @@ -183,17 +187,17 @@ static bool driver_filter(struct device *dev) return ret; } -#define err_printk(dev, entry, format, arg...) do { \ - error_count += 1; \ - if (driver_filter(dev) && \ - (show_all_errors || show_num_errors > 0)) { \ - WARN(1, "%s %s: " format, \ - dev_driver_string(dev), \ - dev_name(dev) , ## arg); \ - dump_entry_trace(entry); \ - } \ - if (!show_all_errors && show_num_errors > 0) \ - show_num_errors -= 1; \ +#define err_printk(dev, entry, format, arg...) do { \ + error_count += 1; \ + if (driver_filter(dev) && \ + (show_all_errors || show_num_errors > 0)) { \ + WARN(1, "%s %s: " format, \ + dev ? dev_driver_string(dev) : "NULL", \ + dev ? dev_name(dev) : "NULL", ## arg); \ + dump_entry_trace(entry); \ + } \ + if (!show_all_errors && show_num_errors > 0) \ + show_num_errors -= 1; \ } while (0); /* -- cgit v1.1 From f4b0373b26567cafd421d91101852ed7a34e9e94 Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Fri, 21 Aug 2009 09:26:15 -0700 Subject: Make bitmask 'and' operators return a result code When 'and'ing two bitmasks (where 'andnot' is a variation on it), some cases want to know whether the result is the empty set or not. In particular, the TLB IPI sending code wants to do cpumask operations and determine if there are any CPU's left in the final set. So this just makes the bitmask (and cpumask) functions return a boolean for whether the result has any bits set. Cc: stable@kernel.org (2.6.30, needed by TLB shootdown fix) Signed-off-by: Linus Torvalds --- lib/bitmap.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) (limited to 'lib') diff --git a/lib/bitmap.c b/lib/bitmap.c index 35a1f7f..7025658 100644 --- a/lib/bitmap.c +++ b/lib/bitmap.c @@ -179,14 +179,16 @@ void __bitmap_shift_left(unsigned long *dst, } EXPORT_SYMBOL(__bitmap_shift_left); -void __bitmap_and(unsigned long *dst, const unsigned long *bitmap1, +int __bitmap_and(unsigned long *dst, const unsigned long *bitmap1, const unsigned long *bitmap2, int bits) { int k; int nr = BITS_TO_LONGS(bits); + unsigned long result = 0; for (k = 0; k < nr; k++) - dst[k] = bitmap1[k] & bitmap2[k]; + result |= (dst[k] = bitmap1[k] & bitmap2[k]); + return result != 0; } EXPORT_SYMBOL(__bitmap_and); @@ -212,14 +214,16 @@ void __bitmap_xor(unsigned long *dst, const unsigned long *bitmap1, } EXPORT_SYMBOL(__bitmap_xor); -void __bitmap_andnot(unsigned long *dst, const unsigned long *bitmap1, +int __bitmap_andnot(unsigned long *dst, const unsigned long *bitmap1, const unsigned long *bitmap2, int bits) { int k; int nr = BITS_TO_LONGS(bits); + unsigned long result = 0; for (k = 0; k < nr; k++) - dst[k] = bitmap1[k] & ~bitmap2[k]; + result |= (dst[k] = bitmap1[k] & ~bitmap2[k]); + return result != 0; } EXPORT_SYMBOL(__bitmap_andnot); -- cgit v1.1 From a30b595d2ca6d39e784a1bed5f2b35f3d7a03af7 Mon Sep 17 00:00:00 2001 From: David Rientjes Date: Wed, 26 Aug 2009 14:29:20 -0700 Subject: flex_array: fix get function for elements in base starting at non-zero If all array elements fit into the base structure and data is copied using flex_array_put() starting at a non-zero index, flex_array_get() will fail to return the data. This fixes the bug by only checking for NULL parts when all elements do not fit in the base structure when flex_array_get() is used. Otherwise, fa_element_to_part_nr() will always be 0 since there are no parts structures needed and such element may never have been put. Thus, it will remain NULL due to the kzalloc() of the base. Additionally, flex_array_put() now only checks for a NULL part when all elements do not fit in the base structure. This is otherwise unnecessary since the base structure is guaranteed to exist (or we would have already hit a NULL pointer). Signed-off-by: David Rientjes Acked-by: Dave Hansen Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- lib/flex_array.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) (limited to 'lib') diff --git a/lib/flex_array.c b/lib/flex_array.c index 08f1636..e73c691a 100644 --- a/lib/flex_array.c +++ b/lib/flex_array.c @@ -198,10 +198,11 @@ int flex_array_put(struct flex_array *fa, int element_nr, void *src, gfp_t flags return -ENOSPC; if (elements_fit_in_base(fa)) part = (struct flex_array_part *)&fa->parts[0]; - else + else { part = __fa_get_part(fa, part_nr, flags); - if (!part) - return -ENOMEM; + if (!part) + return -ENOMEM; + } dst = &part->elements[index_inside_part(fa, element_nr)]; memcpy(dst, src, fa->element_size); return 0; @@ -257,11 +258,12 @@ void *flex_array_get(struct flex_array *fa, int element_nr) if (element_nr >= fa->total_nr_elements) return NULL; - if (!fa->parts[part_nr]) - return NULL; if (elements_fit_in_base(fa)) part = (struct flex_array_part *)&fa->parts[0]; - else + else { part = fa->parts[part_nr]; + if (!part) + return NULL; + } return &part->elements[index_inside_part(fa, element_nr)]; } -- cgit v1.1 From 105b6e8a74cac11cdf70903877593c7f202075cc Mon Sep 17 00:00:00 2001 From: David Rientjes Date: Wed, 26 Aug 2009 14:29:20 -0700 Subject: flex_array: fix flex_array_free_parts comment flex_array_free_parts() does not take `src' or `element_nr' formals, so remove their respective comments. Signed-off-by: David Rientjes Acked-by: Dave Hansen Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- lib/flex_array.c | 3 --- 1 file changed, 3 deletions(-) (limited to 'lib') diff --git a/lib/flex_array.c b/lib/flex_array.c index e73c691a..cf4e372 100644 --- a/lib/flex_array.c +++ b/lib/flex_array.c @@ -122,9 +122,6 @@ static int fa_element_to_part_nr(struct flex_array *fa, int element_nr) /** * flex_array_free_parts - just free the second-level pages - * @src: address of data to copy into the array - * @element_nr: index of the position in which to insert - * the new element. * * This is to be used in cases where the base 'struct flex_array' * has been statically allocated and should not be free. -- cgit v1.1 From b62e408c05228f40e69bb38a48db8961cac6cd23 Mon Sep 17 00:00:00 2001 From: David Rientjes Date: Wed, 26 Aug 2009 14:29:22 -0700 Subject: flex_array: convert element_nr formals to unsigned It's problematic to allow signed element_nr's or total's to be passed as part of the flex array API. flex_array_alloc() allows total_nr_elements to be set to a negative quantity, which is obviously erroneous. flex_array_get() and flex_array_put() allows negative array indices in dereferencing an array part, which could address memory mapped before struct flex_array. The fix is to convert all existing element_nr formals to be qualified as unsigned. Existing checks to compare it to total_nr_elements or the max array size based on element_size need not be changed. Signed-off-by: David Rientjes Cc: Dave Hansen Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- lib/flex_array.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) (limited to 'lib') diff --git a/lib/flex_array.c b/lib/flex_array.c index cf4e372..7baed2f 100644 --- a/lib/flex_array.c +++ b/lib/flex_array.c @@ -99,7 +99,8 @@ static inline int elements_fit_in_base(struct flex_array *fa) * capacity in the base structure. Also note that no effort is made * to efficiently pack objects across page boundaries. */ -struct flex_array *flex_array_alloc(int element_size, int total, gfp_t flags) +struct flex_array *flex_array_alloc(int element_size, unsigned int total, + gfp_t flags) { struct flex_array *ret; int max_size = nr_base_part_ptrs() * __elements_per_part(element_size); @@ -115,7 +116,8 @@ struct flex_array *flex_array_alloc(int element_size, int total, gfp_t flags) return ret; } -static int fa_element_to_part_nr(struct flex_array *fa, int element_nr) +static int fa_element_to_part_nr(struct flex_array *fa, + unsigned int element_nr) { return element_nr / __elements_per_part(fa->element_size); } @@ -143,14 +145,12 @@ void flex_array_free(struct flex_array *fa) kfree(fa); } -static int fa_index_inside_part(struct flex_array *fa, int element_nr) +static unsigned int index_inside_part(struct flex_array *fa, + unsigned int element_nr) { - return element_nr % __elements_per_part(fa->element_size); -} + unsigned int part_offset; -static int index_inside_part(struct flex_array *fa, int element_nr) -{ - int part_offset = fa_index_inside_part(fa, element_nr); + part_offset = element_nr % __elements_per_part(fa->element_size); return part_offset * fa->element_size; } @@ -185,7 +185,8 @@ __fa_get_part(struct flex_array *fa, int part_nr, gfp_t flags) * * Locking must be provided by the caller. */ -int flex_array_put(struct flex_array *fa, int element_nr, void *src, gfp_t flags) +int flex_array_put(struct flex_array *fa, unsigned int element_nr, void *src, + gfp_t flags) { int part_nr = fa_element_to_part_nr(fa, element_nr); struct flex_array_part *part; @@ -217,7 +218,8 @@ int flex_array_put(struct flex_array *fa, int element_nr, void *src, gfp_t flags * * Locking must be provided by the caller. */ -int flex_array_prealloc(struct flex_array *fa, int start, int end, gfp_t flags) +int flex_array_prealloc(struct flex_array *fa, unsigned int start, + unsigned int end, gfp_t flags) { int start_part; int end_part; @@ -248,7 +250,7 @@ int flex_array_prealloc(struct flex_array *fa, int start, int end, gfp_t flags) * * Locking must be provided by the caller. */ -void *flex_array_get(struct flex_array *fa, int element_nr) +void *flex_array_get(struct flex_array *fa, unsigned int element_nr) { int part_nr = fa_element_to_part_nr(fa, element_nr); struct flex_array_part *part; -- cgit v1.1 From 4f8ee2c9cc0e885d2bb50ef26db66150ab25213e Mon Sep 17 00:00:00 2001 From: Benjamin Herrenschmidt Date: Thu, 27 Aug 2009 17:20:30 +1000 Subject: lmb: Remove __init from lmb_end_of_DRAM() We call lmb_end_of_DRAM() to test whether a DMA mask is ok on a machine without IOMMU, but this function is marked as __init. I don't think there's a clean way to get the top of RAM max_pfn doesn't appear to include highmem or I missed (or we have a bug :-) so for now, let's just avoid having a broken 2.6.31 by making this function non-__init and we can revisit later. Signed-off-by: Benjamin Herrenschmidt Signed-off-by: Linus Torvalds --- lib/lmb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/lmb.c b/lib/lmb.c index e4a6482..0343c05 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -429,7 +429,7 @@ u64 __init lmb_phys_mem_size(void) return lmb.memory.size; } -u64 __init lmb_end_of_DRAM(void) +u64 lmb_end_of_DRAM(void) { int idx = lmb.memory.cnt - 1; -- cgit v1.1