Re: [PATCH] fix qsort breakage

[Christoph Hellwig <hch@infradead.org>]

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)