NULL pointer deref in soc/intel/skylake/irq.c

[Benjamin Doron]

Hi all,
`src/soc/intel/skylake/irq.c:soc_irq_settings()` dereferences NULL by memcpy'ing devintconfig to params->DevIntConfigPtr. As far as I can tell, this happens because we're copying a structure onto a UPD that's actually just the pointer, rather than assigning the UPD to the buffer.

devintconfig is static, so will `params->DevIntConfigPtr = devintconfig` work? It would point inside the data section, as I understand. Alternatively, calling `malloc()` first should work.

I'm away from my computer now, so I can't test these alternatives yet.

Best regards,
Benjamin

[Angel Pons]

Hi Benjamin,

On Wed, Jun 15, 2022 at 9:23 PM Benjamin Doron
 wrote:
> Hi all,
> `src/soc/intel/skylake/irq.c:soc_irq_settings()` dereferences NULL by memcpy'ing devintconfig to params->DevIntConfigPtr. As far as I can tell, this happens because we're copying a structure onto a UPD that's actually just the pointer, rather than assigning the UPD to the buffer.

Yes, this code stinks. It makes no sense, where does `DevIntConfigPtr`
originally point to? It might not even be pointing to RAM depending on
how much RAM is installed!
> devintconfig is static, so will `params->DevIntConfigPtr = devintconfig` work? It would point inside the data section, as I understand. Alternatively, calling `malloc()` first should work.

Newer platforms simply do an assignment (with some casts) so I expect
this to work. They also use a dynamically-allocated buffer because the
data is generated at runtime from a coreboot-specific structure.

I made https://review.coreboot.org/65217 but I haven't boot-tested it.
> I'm away from my computer now, so I can't test these alternatives yet.
>
> Best regards,
> Benjamin
> _______________________________________________
> coreboot mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]

Best regards,
Angel