On Sat, May 04, 2002 at 12:15:12PM +0100, Christoph Hellwig wrote:
> Qsort in the XFS tree has two problems:
>
> o allocates memory using GFP_KERNEL although called from under i_sem
> (possible deadlock)
> o doesn't check kmalloc return value (possible NULL-ptr dereference)
>
> The below patch tries to address both issues, but without a return value
> singnalling ENOMEM is rather difficult..
>
> Andi Kleen suggested getting the pivot from stack, someone with enough
> time might check the callers for sane ßize arguments.
Wessel Dankers found another bug in the qsort implementation, a memory
leak is possible if total_elems is zero. Updated patch is below.
The alternative would be to replace this masterpiece of GNU bloatware with
a simpler qsort implementation that doesn't use dynamic memory allocation
like the one in 4BSD's libc or the public domain one from:
http://www.snippets.org/snippets/portable/RG_QSORT+C.php3
Of course you could take the IRIX kernel one as well - it should fully
match the assupmtions XFS makes, I guess..
Christoph
Index: linux/fs/xfs_support/qsort.c
===================================================================
RCS file: /cvs/linux-2.4-xfs/linux/fs/xfs_support/qsort.c,v
retrieving revision 1.4
diff -u -u -r1.4 qsort.c
--- linux/fs/xfs_support/qsort.c 2002/03/12 06:25:01 1.4
+++ linux/fs/xfs_support/qsort.c 2002/05/04 13:28:18
@@ -85,14 +85,18 @@
int (*cmp)(const void *, const void *))
{
register char *base_ptr = (char *) pbase;
-
- /* Allocating SIZE bytes for a pivot buffer facilitates a better
- algorithm below since we can do comparisons directly on the pivot. */
- char *pivot_buffer = (char *) kmalloc (size, GFP_KERNEL);
const size_t max_thresh = MAX_THRESH * size;
-
+ char *pivot_buffer;
+
if (total_elems == 0)
/* Avoid lossage with unsigned arithmetic below. */
+ return;
+
+ /* Allocating SIZE bytes for a pivot buffer facilitates a better
+ algorithm below since we can do comparisons directly on the pivot. */
+ pivot_buffer = kmalloc (size, GFP_NOFS);
+ if (pivot_buffer == NULL)
+ /* <shrug> any way to return failure from qsort? */
return;
if (total_elems > MAX_THRESH)
|