Re: [Freeipmi-devel] [RFC][PATCH 0/1] Fix apparent bug w/ ACPI SPMI table parsing

[dann frazier]

On Mon, Jul 30, 2018 at 03:30:44PM -0600, dann frazier wrote:
> hey.
>   I'm working on a patch to add support to ipmi-locate code to parse ACPI
> SPMI tables via sysfs. This is to make ipmi-locate work on an ARM system
> we have that describes its BMC only in ACPI (not DMI). Being ARM, trolling
> /dev/mem isn't safe, (freeipmi ifdef's that out), so using the kernel-exposed
> tables, when available, is a better option.
> 
> While doing this I ran across what seems like a bug, which the following patch
> should fix. I say "seems" and "should" because I don't have a system where the
> existing /dev/mem snooping code works - with, or without this bug fix -
> therefore, I cannot emperically demonstrate the bug.

I am able to *simulate* the issue with the following patch:

diff --git a/libfreeipmi/locate/ipmi-locate-acpi-spmi.c 
b/libfreeipmi/locate/ipmi-locate-acpi-spmi.c
index 13c0e30cc..3b8ee98fb 100644
--- a/libfreeipmi/locate/ipmi-locate-acpi-spmi.c
+++ b/libfreeipmi/locate/ipmi-locate-acpi-spmi.c
@@ -1189,12 +1189,45 @@ _ipmi_acpi_get_firmware_table (ipmi_locate_ctx_t ctx,
       { 0, "", 0}
     };
 
+  /*  const uint8_t test_spmi_data[] = { 0x50, 0x53, 0x49, 0x4d, 0x00, 0x41,
+                                    0x00, 0x00, 0xd7, 0x05, 0x49, 0x48,
+                                    0x49, 0x53, 0x20, 0x20, 0x49, 0x48,
+                                    0x30, 0x50, 0x20, 0x38, 0x20, 0x20,
+                                    0x00, 0x00, 0x00, 0x00, 0x4e, 0x49,
+                                    0x4c, 0x54, 0x11, 0x24, 0x20, 0x15,
+                                    0x01, 0x03, 0x02, 0x00, 0x00, 0x00,
+                                    0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                                    0x08, 0x00, 0x01, 0x00, 0x00, 0xe4,
+                                    0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                                    0x00, 0x00, 0x00, 0x00 };
+  */
+  const uint8_t test_spmi_data[] = { 0x53, 0x50, 0x4d, 0x49, 0x41, 0x00,
+                                    0x00, 0x00, 0x05, 0xd7, 0x48, 0x49,
+                                    0x53, 0x49, 0x20, 0x20, 0x48, 0x49,
+                                    0x50, 0x30, 0x38, 0x20, 0x20, 0x20,
+                                    0x00, 0x00, 0x00, 0x00, 0x49, 0x4e,
+                                    0x54, 0x4c, 0x24, 0x11, 0x15, 0x20,
+                                    0x03, 0x01, 0x00, 0x02, 0x00, 0x00,
+                                    0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                                    0x00, 0x08, 0x00, 0x01, 0xe4, 0x00,
+                                    0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                                    0x00, 0x00, 0x00, 0x00 };
+  const uint32_t test_spmi_data_length = 64;
+
   assert (ctx);
   assert (ctx->magic == IPMI_LOCATE_CTX_MAGIC);
   assert (signature);
   assert (sign_table_data);
   assert (sign_table_data_length);
 
+  if ((strcmp ("SPMI", signature)) == 0)
+    {
+      acpi_table = malloc(test_spmi_data_length);
+      memcpy(acpi_table, test_spmi_data, test_spmi_data_length);
+      acpi_table_length = test_spmi_data_length;
+      goto fakespmi;
+    }
+  
   *sign_table_data = NULL;
 
   if ((acpi_table_hdr_length = fiid_template_len_bytes (tmpl_acpi_table_hdr)) 
< 0)
@@ -1332,6 +1365,7 @@ _ipmi_acpi_get_firmware_table (ipmi_locate_ctx_t ctx,
       acpi_table_length = 0;
     }
 
+ fakespmi:
   if (!acpi_table)
     {
       LOCATE_SET_ERRNUM (ctx, IPMI_LOCATE_ERR_SYSTEM_ERROR);
-- 
2.18.0