Hello, Almudena Garcia, le lun. 21 sept. 2026 21:28:41 +0200, a ecrit: > 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)
In the current state of the code, I agree. But if someday we implement x2APIC, we'll be happy that the code is already ready for much larger IDs. Samuel > > > 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); > > -- Samuel /* Amuse the user. */ printk( " \\|/ ____ \\|/\n" " \"@'/ ,. \\`@\"\n" " /_| \\__/ |_\\\n" " \\__U_/\n"); (From linux/arch/sparc/kernel/traps.c:die_if_kernel())
