Hello: I am the original writer of this code. In the original code, the list reserve capacity for the max number of cpus (265, max allowed by xAPIC) and, once we know how many cpus are really in the machine, then reduce the capacity store this amount. By this reason we don't check the capacity: it's already the maximum allowed by the standard.
The APIC ID never can be over than 255 (the APIC ID use 8 bits) El lun, 21 sept 2026 a las 20:26, David Bidner (<[email protected]>) escribió: > apic_add_cpu() wrote cpu_lapic_list[ncpus] without checking the > allocation size. Keep the allocated size in capacity, return -1 when the > list is full, and bound apic_get_cpu_apic_id() and the refit copy/free by > it. Reject APIC IDs above 255 in apic_get_cpu_kernel_id(). > > cpu_id_lut now starts at -1, so an APIC ID that was not accepted is no > longer mapped to the bootstrap processor. > --- > i386/i386/apic.c | 29 +++++++++++++++++++++++++---- > i386/i386/apic.h | 3 ++- > 2 files changed, 27 insertions(+), 5 deletions(-) > > diff --git a/i386/i386/apic.c b/i386/i386/apic.c > index a53992c..a841bd5 100644 > --- a/i386/i386/apic.c > +++ b/i386/i386/apic.c > @@ -61,10 +61,13 @@ uint8_t apic_id_mask = 0xf; > int > apic_data_init(void) > { > + int i; > + > apic_data.cpu_lapic_list = NULL; > apic_data.ncpus = 0; > apic_data.nioapics = 0; > apic_data.nirqoverride = 0; > + apic_data.capacity = 0; > > /* Reserve the vector memory for the maximum number of processors. */ > apic_data.cpu_lapic_list = (uint16_t*) kalloc(NCPUS*sizeof(uint16_t)); > @@ -73,6 +76,12 @@ apic_data_init(void) > if (apic_data.cpu_lapic_list == NULL) > return -1; > > + apic_data.capacity = NCPUS; > + > + /* -1 marks an APIC ID that was not accepted. */ > + for (i = 0; i <= UINT8_MAX; i++) > + cpu_id_lut[i] = -1; > + > return 0; > } > > @@ -89,12 +98,18 @@ apic_lapic_init(ApicLocalUnit* lapic_ptr) > /* > * apic_add_cpu: add a new lapic/cpu entry to the cpu_lapic list. > * Receives as input the lapic's APIC ID. > + * Returns 0 if accepted, -1 if the list is full. > */ > -void > +int > apic_add_cpu(uint16_t apic_id) > { > + if (apic_data.ncpus >= apic_data.capacity) > + return -1; > + > apic_data.cpu_lapic_list[apic_data.ncpus] = apic_id; > apic_data.ncpus++; > + > + return 0; > } > > /* > @@ -139,7 +154,7 @@ acpi_get_irq_override(uint8_t pin) > int > apic_get_cpu_apic_id(int kernel_id) > { > - if (kernel_id >= NCPUS) > + if (kernel_id < 0 || kernel_id >= apic_data.capacity) > return -1; > > return apic_data.cpu_lapic_list[kernel_id]; > @@ -153,6 +168,9 @@ apic_get_cpu_apic_id(int kernel_id) > int > apic_get_cpu_kernel_id(uint16_t apic_id) > { > + if (apic_id > UINT8_MAX) > + return -1; > + > return cpu_id_lut[apic_id]; > } > > @@ -227,8 +245,10 @@ int apic_refit_cpulist(void) > { > uint16_t* old_list = apic_data.cpu_lapic_list; > uint16_t* new_list = NULL; > + uint16_t old_capacity = apic_data.capacity; > > - if (old_list == NULL) > + if (old_list == NULL || apic_data.ncpus == 0 > + || apic_data.ncpus > old_capacity) > return -1; > > new_list = (uint16_t*) kalloc(apic_data.ncpus*sizeof(uint16_t)); > @@ -240,7 +260,8 @@ int apic_refit_cpulist(void) > new_list[i] = old_list[i]; > > apic_data.cpu_lapic_list = new_list; > - kfree((vm_offset_t) old_list, NCPUS*sizeof(uint16_t)); > + apic_data.capacity = apic_data.ncpus; > + kfree((vm_offset_t) old_list, old_capacity*sizeof(uint16_t)); > > return 0; > } > diff --git a/i386/i386/apic.h b/i386/i386/apic.h > index df95b81..7b6fcea 100644 > --- a/i386/i386/apic.h > +++ b/i386/i386/apic.h > @@ -230,6 +230,7 @@ typedef struct ApicInfo { > uint8_t ncpus; > uint8_t nioapics; > int nirqoverride; > + uint16_t capacity; > uint16_t* cpu_lapic_list; > struct IoApicData ioapic_list[MAX_IOAPICS]; > struct IrqOverrideData irq_override_list[MAX_IRQ_OVERRIDE]; > @@ -241,7 +242,7 @@ struct irqinfo { > }; > > int apic_data_init(void); > -void apic_add_cpu(uint16_t apic_id); > +int apic_add_cpu(uint16_t apic_id); > void apic_lapic_init(ApicLocalUnit* lapic_ptr); > void apic_add_ioapic(struct IoApicData); > void apic_add_irq_override(struct IrqOverrideData irq_over); > >
