Re: [patch #3346] GNUMach: ifdef DEBUG -> ifndef NDEBUG

[Alfred M. Szmidt]

   Sounds like it might be this [1] patch.

Thanks, I inlined it.

Roland, OK to commit?  Then I can commit the previous patch with a
good concious; after I test it obviously...


Summary:  Double free and memory loss probing partition table

Original Submission: While GNU Mach reads the partition table, the
second assert in linux/dev/glue:free_pages is triggered. This
particular assert checks for double frees.  I have traced the problem
back to getblk and __brelse: if linux_auto_config is true (which it is
when partitions are being probed), a static buffer is used to hold the
BH structure. If getblk is called a second time (i.e. before the first
block is released), the buffer is overriden.  This results in a double
free, a memory leak (as the buffer in the first BH is never released)
and a consistency problem as code which uses the first buffer will now
see different data. This is the case in
linux/dev/drivers/block/genhd.c:msdos_partition which calls bread
then, before freeing the block, calls extended_partition which also
calls bread. In reality, there is no reason to not use kalloc and
kfree here. In kern/statup.c:setup_main, we see that vm_mem_bootstrap
which calls kmem_init is called long before
linux/dev/init/main.c:linux_init is invoked by
i386/i386at/machine_init:machine_init.

This attached patch changes getblk and __brelse to always use kalloc
and kfree and adds asserts to kern/kalloc.c to make sure that kalloc,
kfree and kget are only called after kmem_init has been called.

Apply the patch using -p0

2004-09-07  Neal H. Walfield  <neal@cs.uml.edu>

        * linux/dev/glue/block.c (__brelse): Unconditionally kfree BH.
        (getblk): Unconditionally kalloc BH.

        * kern/kalloc.c [!NDEBUG] (kalloc_init_called): New static
        variable.
        (kalloc_init): Assert that kalloc_init_called is zero.
        [! NDEBUG] Set kalloc_init_called to 1 on success.
        (kalloc): Assert that kalloc_init_called is non-zero.
        (kget): Likewise.
        (kfree): Likewise.

Index: linux/dev/glue/block.c
===================================================================
RCS file: /cvsroot/hurd/gnumach/linux/dev/glue/Attic/block.c,v
retrieving revision 1.8.2.2
diff -u -p -r1.8.2.2 block.c
--- linux/dev/glue/block.c      19 Jan 2004 01:44:31 -0000      1.8.2.2
+++ linux/dev/glue/block.c      7 Sep 2004 15:08:17 -0000
@@ -354,22 +354,17 @@ struct buffer_head *
 getblk (kdev_t dev, int block, int size)
 {
   struct buffer_head *bh;
-  static struct buffer_head bhead;
 
   assert (size <= PAGE_SIZE);
 
-  if (! linux_auto_config)
-    bh = (struct buffer_head *) kalloc (sizeof (struct buffer_head));
-  else
-    bh = &bhead;
+  bh = (struct buffer_head *) kalloc (sizeof (struct buffer_head));
   if (bh)
     {
       memset (bh, 0, sizeof (struct buffer_head));
       bh->b_data = alloc_buffer (size);
       if (! bh->b_data)
        {
-         if (! linux_auto_config)
-           kfree ((vm_offset_t) bh, sizeof (struct buffer_head));
+         kfree ((vm_offset_t) bh, sizeof (struct buffer_head));
          return NULL;
        }
       bh->b_dev = dev;
@@ -385,8 +380,7 @@ void
 __brelse (struct buffer_head *bh)
 {
   free_buffer (bh->b_data, bh->b_size);
-  if (! linux_auto_config)
-    kfree ((vm_offset_t) bh, sizeof (*bh));
+  kfree ((vm_offset_t) bh, sizeof (*bh));
 }
 
 /* Allocate a buffer of SIZE bytes and fill it with data
Index: kern/kalloc.c
===================================================================
RCS file: /cvsroot/hurd/gnumach/kern/kalloc.c,v
retrieving revision 1.1.1.1
diff -u -p -r1.1.1.1 kalloc.c
--- kern/kalloc.c       25 Feb 1997 21:28:23 -0000      1.1.1.1
+++ kern/kalloc.c       7 Sep 2004 15:08:17 -0000
@@ -106,12 +106,18 @@ unsigned long k_zone_max[16] = {
  *     This initializes all of the zones.
  */
 
+#ifndef NDEBUG
+static int kalloc_init_called;
+#endif
+
 void kalloc_init()
 {
        vm_offset_t min, max;
        vm_size_t size;
        register int i;
 
+       assert (! kalloc_init_called);
+
        kalloc_map = kmem_suballoc(kernel_map, &min, &max,
                                   kalloc_map_size, FALSE);
 
@@ -142,6 +148,10 @@ void kalloc_init()
                                  size >= PAGE_SIZE ? ZONE_COLLECTABLE : 0,
                                  k_zone_name[i]);
        }
+
+#ifndef NDEBUG
+       kalloc_init_called = 1;
+#endif
 }
 
 vm_offset_t kalloc(size)
@@ -153,6 +163,8 @@ vm_offset_t kalloc(size)
 
        /* compute the size of the block that we will actually allocate */
 
+       assert (kalloc_init_called);
+
        allocsize = size;
        if (size < kalloc_max) {
                allocsize = MINSIZE;
@@ -185,6 +197,8 @@ vm_offset_t kget(size)
        register vm_size_t allocsize;
        vm_offset_t addr;
 
+       assert (kalloc_init_called);
+
        /* compute the size of the block that we will actually allocate */
 
        allocsize = size;
@@ -219,6 +233,8 @@ kfree(data, size)
        register int zindex;
        register vm_size_t freesize;
 
+       assert (kalloc_init_called);
+
        freesize = size;
        if (size < kalloc_max) {
                freesize = MINSIZE;