serious mem overflow in mpbios(4) plz test

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

serious mem overflow in mpbios(4) plz test

Michael Shalayeff-2
re
there is a completely bogus assumption made in the original mpbios(4)
implementation that assumes one particualr order on the entries in it.
this leads to a scenario where interrupt (both iopic and lopic) tables
get much lower sizes than needed (by a factor of two) thus trashing
some other memory later or (even worse) being trashed 'emselves and
thus causing totally bogus apic programming later. the previous
change "accidentaly" fixes this problem but here is the real fix.
therefore
everybody who had any kinds of problems w/ smp and interrupts plz test.
cu

--
    paranoic mickey       (my employers have changed but, the name has remained)


Index: mpbios.c
===================================================================
RCS file: /cvs/src/sys/arch/i386/i386/mpbios.c,v
retrieving revision 1.6
diff -u -r1.6 mpbios.c
--- mpbios.c 23 Nov 2005 09:24:46 -0000 1.6
+++ mpbios.c 28 Nov 2005 15:38:12 -0000
@@ -576,28 +576,39 @@
  /*
  * Walk the table once, counting items
  */
- position = (const u_int8_t *)(mp_cth);
- end = position + mp_cth->base_len;
- position += sizeof(*mp_cth);
+ for (count = mp_cth->entry_count,
+    position = (const u_int8_t *)mp_cth + sizeof(*mp_cth),
+    end = position + mp_cth->base_len;
+    count-- && position < end;
+    position += mp_conf[type].length) {
 
- count = mp_cth->entry_count;
- intr_cnt = 15; /* presume all isa irqs being missing */
-
- while ((count--) && (position < end)) {
  type = *position;
  if (type >= MPS_MCT_NTYPES) {
  printf("%s: unknown entry type %x"
     " in MP config table\n",
     self->dv_xname, type);
+ end = position;
  break;
  }
  mp_conf[type].count++;
+ }
+
+ /*
+ * Walk the table twice, counting int and bus entries
+ */
+ for (count = mp_cth->entry_count,
+    intr_cnt = 15, /* presume all isa irqs missing */
+    position = (const u_int8_t *)mp_cth + sizeof(*mp_cth);
+    count-- && position < end;
+    position += mp_conf[type].length) {
+ type = *position;
  if (type == MPS_MCT_BUS) {
  const struct mpbios_bus *bp =
     (const struct mpbios_bus *)position;
  if (bp->bus_id >= mp_nbus)
  mp_nbus = bp->bus_id + 1;
  }
+
  /*
  * Count actual interrupt instances.
  * dst_apic_id of MPS_ALL_APICS means "wired to all
@@ -615,7 +626,6 @@
  else
  intr_cnt += mp_conf[MPS_MCT_CPU].count;
  }
- position += mp_conf[type].length;
  }
 
  mp_busses = malloc(sizeof(struct mp_bus) * mp_nbus,

Reply | Threaded
Open this post in threaded view
|

Re: serious mem overflow in mpbios(4) plz test

Marco S Hyman
mickey writes:
 > there is a completely bogus assumption made in the original mpbios(4)
 > implementation that assumes one particualr order on the entries in it.

The patch is only for i386.  Is this a problem with amd64, too?

// marc

Reply | Threaded
Open this post in threaded view
|

Re: serious mem overflow in mpbios(4) plz test

Michael Shalayeff-2
On Tue, Nov 29, 2005 at 06:47:20PM -0800, Marco S Hyman wrote:
> mickey writes:
>  > there is a completely bogus assumption made in the original mpbios(4)
>  > implementation that assumes one particualr order on the entries in it.
>
> The patch is only for i386.  Is this a problem with amd64, too?

amd64 code has some other differences so maybe it makes sense
to make a separate patch for it then...

cu
--
    paranoic mickey       (my employers have changed but, the name has remained)