Comment 20 for bug 1875771

Revision history for this message
Bryce Harrington (bryce) wrote :

1260: static int
1261: _ipmi_acpi_get_table_dev_mem (ipmi_locate_ctx_t ctx,
1262: char *signature,
1263: unsigned int table_instance,
1264: uint8_t **acpi_table,
1265: uint32_t *acpi_table_length)
1266: {
...
1305: assert (acpi_table);
1306: assert (acpi_table_length);
1307:
1308: *acpi_table = NULL;
...
1387: acpi_table = NULL;
1388: acpi_table_length = 0;
1389: for (i = 0, signature_table_count = 0; i < acpi_table_count; i++)
1340: {
...
1429: if (_ipmi_acpi_get_table (ctx,
1430: table_address,
1431: signature,
1432: acpi_table,
1433: acpi_table_length) < 0)
1434: continue;
...
1440: free (acpi_table);
1441: acpi_table = NULL;
1442: acpi_table_length = 0;
1443: }

_ipmi_acpi_get_table() is documented as requiring malloc'd memory passed in via its acpi_table argument, and in fact asserts that it's non null before using it. So passing acpi_table=NULL is a programming error, yet it appears this is what happens via line 1387.

I wonder if perhaps what was meant on line 1387 was:

1387: *acpi_table = NULL;

If it was, that seems redundant with line 1308 so still seems odd. In any case, setting acpi_table = NULL and then passing that to _ipmi_acpi_get_table() seems very suspect. It might be interesting to see what would happen if you try commenting out line 1387 and trying to reproduce the crash? It looks like this code was added in 0.7.15-1 (Nov 2009).