Mysterious memory corruption bug

* Mysterious memory corruption bug
@ 2012-05-01 18:53 Bean
  2012-05-01 19:08 ` Vladimir 'φ-coder/phcoder' Serbinenko
  0 siblings, 1 reply; 12+ messages in thread
From: Bean @ 2012-05-01 18:53 UTC (permalink / raw)
  To: The development of GRUB 2

[-- Attachment #1: Type: text/plain, Size: 1475 bytes --]

Hi,

Thanks to Vladimir's memory patch, it's actually quite easy to
reproduce mysterious issue.

First, there are two memory leaks in ip.c.

It allocates the rsm but never frees it. free_rsm frees its content,
but not the pointer itself. You can see it in printmem at ip.c:473
      rsm = grub_malloc (sizeof (*rsm));

Another problem is at ip.c:594:
  return handle_dgram (ret, card, src_hwaddress,
			       hwaddress, proto, &source, &dest,
			       ttl);
here, ret is netbuff. grub_netbuff_alloc get a buffer for both data
and header (data go first), so when it frees the data pointer, the
header goes away as well. But here, the header is allocated separately
so that it's not free using , you can see it from printmem at ip.c:580
      ret = grub_malloc (sizeof (*ret));

Now here's the tricky part, when i fix both problem, it actually when
you call this: (memdisk size is 19,180, just in case it matters).

testspeed /memdisk

So there must be a memory corruption somewhere. (It will not halt if
you skip the the second leak, but you can see the remaining buffer in
printmem).

BTW, you should add a grub_free_fragment call in testspeed to free the
rsm cache, just to make the printmem output a little cleaner.

These are the modules used to generate grub.efi, just in case it's relevant.

/grub-mkimage -d grub-core -o grub.efi -O x86_64-efi chain boot test
fat ntfs part_msdos normal ls echo efinet tftp http efinet reboot
testspeed printmem

-- 
Best wishes
Bean

[-- Attachment #2: test.txt --]
[-- Type: text/plain, Size: 3085 bytes --]

=== modified file 'grub-core/net/ip.c'

--- grub-core/net/ip.c	2012-02-09 22:43:43 +0000
+++ grub-core/net/ip.c	2012-05-01 18:28:59 +0000
@@ -240,7 +240,7 @@
 	FOR_NET_NETWORK_LEVEL_INTERFACES (inf)
 	  if (inf->card == card
 	      && inf->address.type == GRUB_NET_NETWORK_LEVEL_PROTOCOL_DHCP_RECV
-	      && grub_net_hwaddr_cmp (&inf->hwaddress, hwaddress) == 0)
+	      && inf->hwaddress.type == GRUB_NET_LINK_LEVEL_PROTOCOL_ETHERNET)
 	    {
 	      if (udph->chksum)
 		{
@@ -257,18 +257,25 @@
 				    "Expected %x, got %x\n", 
 				    grub_be_to_cpu16 (expected),
 				    grub_be_to_cpu16 (chk));
-		      grub_netbuff_free (nb);
-		      return GRUB_ERR_NONE;
+		      break;
 		    }
 		  udph->chksum = chk;
 		}
 
 	      err = grub_netbuff_pull (nb, sizeof (*udph));
 	      if (err)
-		return err;
-	      grub_net_process_dhcp (nb, inf->card);
-	      grub_netbuff_free (nb);
-	      return GRUB_ERR_NONE;
+		{
+		  grub_netbuff_free (nb);
+		  return err;
+		}
+	      struct grub_net_bootp_packet *dhcp =
+		(struct grub_net_bootp_packet *) nb->data;
+	      if (grub_memcmp(inf->hwaddress.mac, &dhcp->mac_addr,
+			      sizeof(inf->hwaddress.mac)) == 0)
+		{
+		  grub_net_process_dhcp (nb, inf->card);
+		  break;
+		}
 	    }
 	grub_netbuff_free (nb);
 	return GRUB_ERR_NONE;
@@ -344,19 +351,34 @@
     }
   grub_free (rsm->asm_buffer);
   grub_priority_queue_destroy (rsm->pq);
+  grub_free (rsm);
 }
 
 static void
 free_old_fragments (void)
 {
-  struct reassemble *rsm, **prev;
+  struct reassemble *rsm, **prev, *tmp;
   grub_uint64_t limit_time = grub_get_time_ms () - 90000;
 
-  for (prev = &reassembles, rsm = *prev; rsm; prev = &rsm->next, rsm = *prev)
+  for (prev = &reassembles, rsm = *prev; rsm;
+       prev = &rsm->next, rsm = *prev, free_rsm (tmp))
     if (rsm->last_time < limit_time)
       {
 	*prev = rsm->next;
-	free_rsm (rsm);
+	tmp = rsm;	
+      }
+}
+
+void
+grub_free_fragments (void)
+{
+  struct reassemble *rsm, **prev, *tmp;
+
+  for (prev = &reassembles, rsm = *prev; rsm;
+       prev = &rsm->next, rsm = *prev, free_rsm (tmp))
+      {
+	*prev = rsm->next;
+	tmp = rsm;
       }
 }
 
@@ -570,9 +592,14 @@
       dest.type = GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV4;
       dest.ipv4 = dst;
 
-      return handle_dgram (ret, card, src_hwaddress,
-			   hwaddress, proto, &source, &dest,
-			   ttl);
+      {
+	grub_err_t result;	
+	result = handle_dgram (ret, card, src_hwaddress,
+			       hwaddress, proto, &source, &dest,
+			       ttl);
+	grub_free (ret);
+	return result;
+      }
     }
 }
 

=== modified file 'grub-core/net/tftp.c'
--- grub-core/net/tftp.c	2012-02-12 18:11:06 +0000
+++ grub-core/net/tftp.c	2012-05-01 17:22:35 +0000
@@ -320,9 +320,9 @@
   rrqlen += grub_strlen ("blksize") + 1;
   rrq += grub_strlen ("blksize") + 1;
 
-  grub_strcpy (rrq, "1024");
-  rrqlen += grub_strlen ("1024") + 1;
-  rrq += grub_strlen ("1024") + 1;
+  grub_strcpy (rrq, "4096");
+  rrqlen += grub_strlen ("4096") + 1;
+  rrq += grub_strlen ("4096") + 1;
 
   grub_strcpy (rrq, "tsize");
   rrqlen += grub_strlen ("tsize") + 1;


^ permalink raw reply	[flat|nested] 12+ messages in thread