Hey Christian, I've applied the patch, thanks! Just wondering how bad this is and if it warrants a FreeIPMI release. It sounds like on certain systems, a segfault was probably happening all the time?
Al On Thu, 2020-09-03 at 02:50 -0700, Al Chu wrote: > Hi Christian, > > LGTM, thanks and thanks for the thorough description. I'm not a > computer that I can apply right now, but will do so later. > > This seems like a critical enough bug to do a new release, i will try > to do so by the end of the week. > > Al > > On Thu, 2020-09-03 at 09:14 +0200, Christian Ehrhardt wrote: > > Since 68ed819 "acpi-spmi locate driver" the initialization of > > acpi_table > > was done as a direct assignment on a global variable. > > Later on _ipmi_acpi_get_table was restructured and now expects > > malloced > > memory, but the old intialization was left in place now killing the > > malloced > > pointer. > > > > It turns out that on systems without /sys/firmware/acpi/tables/SPMI > > like > > Dell iDRAC6/9 _ipmi_acpi_get_table then tries to assign > > acpi_table_buf to it. > > This leads to an access of 0x0 and a crash, for a backtrace see: > > https://launchpadlibrarian.net/495892193/apport-retrace-ipmi-locate.txt > > > > On other hardware (with SPMI file in sysfs) there are some early > > exits before > > this happens. Additionally Before 094cd5ce "Audit libfreeipmi, > > remove > > unnecessary memsets, fix +1 buffers, etc." which is in >1.6.0 there > > was > > another early exit in _ipmi_acpi_get_table_dev_mem as > > _ipmi_acpi_get_rsdp > > never returned non zero, but since that fix it can reach the bad > > code. > > > > The old style assignment of acpi_table is wrong (writes the pointer > > instead > > of the pointer target) and superfluous as it is done correct at the > > beginning > > of _ipmi_acpi_get_table_dev_mem and between this init and the usage > > no more > > touched. Therefore it is safe to just remove the bad > > initialization. > > > > Signed-off-by: Christian Ehrhardt <christian.ehrha...@canonical.com > > > > > Reported-by: jeffrey.l...@canonical.com > > --- > > libfreeipmi/locate/ipmi-locate-acpi-spmi.c | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/libfreeipmi/locate/ipmi-locate-acpi-spmi.c > > b/libfreeipmi/locate/ipmi-locate-acpi-spmi.c > > index 710674e53..11c688524 100644 > > --- a/libfreeipmi/locate/ipmi-locate-acpi-spmi.c > > +++ b/libfreeipmi/locate/ipmi-locate-acpi-spmi.c > > @@ -1384,7 +1384,6 @@ _ipmi_acpi_get_table_dev_mem > > (ipmi_locate_ctx_t > > ctx, > > else > > acpi_table_count = rsdt_xsdt_table_data_length / 8; > > > > - acpi_table = NULL; > > acpi_table_length = 0; > > for (i = 0, signature_table_count = 0; i < acpi_table_count; > > i++) > > { > >