Skip to contentSkip to conversation

acpi/nfit: Add support for Intel DSM 1.8 commands

bug report Dan Carpenter 1 participant 1 message May 2, 2019

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

Hello Dan Williams,

Commit 11189c1089da ("acpi/nfit: Fix command-supported detection") from Jan 19, 2019 leads to the following static checker warning:

drivers/acpi/nfit/core.c:503 acpi_nfit_ctl()
error: passing untrusted data 'func' to 'variable_test_bit()'

Related but not critical:

drivers/acpi/nfit/core.c:3510 acpi_nfit_clear_to_send()
error: undefined (user controlled) shift '1 << func'

drivers/nvdimm/bus.c
1062 buf = vmalloc(buf_len);
1063 if (!buf)
1064 return -ENOMEM;
1065
1066 if (copy_from_user(buf, p, buf_len)) {
1067 rc = -EFAULT;
1068 goto out;
1069 }
1070

  1071          nvdimm_bus_lock(&nvdimm_bus->dev);
  1072          rc = nd_cmd_clear_to_send(nvdimm_bus, nvdimm, func, buf);
  1073          if (rc)

1074 goto out_unlock;
1075
1076 rc = nd_desc->ndctl(nd_desc, nvdimm, cmd, buf, buf_len, &cmd_rc);
1077 if (rc < 0)
1078 goto out_unlock;

This is __nd_ioctl(). We get "buf" from the user and then pass it to acpi_nfit_clear_to_send() and then acpi_nfit_ctl().

drivers/acpi/nfit/core.c
446 int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm,
447 unsigned int cmd, void *buf, unsigned int buf_len, int *cmd_rc)
448 {

   449          struct acpi_nfit_desc *acpi_desc = to_acpi_desc(nd_desc);
   450          struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
   451          union acpi_object in_obj, in_buf, *out_obj;
   452          const struct nd_cmd_desc *desc = NULL;
   453          struct device *dev = acpi_desc->dev;
   454          struct nd_cmd_pkg *call_pkg = NULL;
   455          const char *cmd_name, *dimm_name;
   456          unsigned long cmd_mask, dsm_mask;
   457          u32 offset, fw_status = 0;
   458          acpi_handle handle;
   459          const guid_t *guid;
   460          int func, rc, i;
   461  
   462          if (cmd_rc)
   463                  *cmd_rc = -EINVAL;
   464  
   465          if (cmd == ND_CMD_CALL)
   466                  call_pkg = buf;
                        ^^^^^^^^^^^^^^
   467          func = cmd_to_func(nfit_mem, cmd, call_pkg);
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

We set func = call_pkg->nd_command (0-s32max).

   468          if (func < 0)
   469                  return func;
   470  
   471          if (nvdimm) {
   472                  struct acpi_device *adev = nfit_mem->adev;
   473  
   474                  if (!adev)
   475                          return -ENOTTY;
   476  
   477                  dimm_name = nvdimm_name(nvdimm);
   478                  cmd_name = nvdimm_cmd_name(cmd);
   479                  cmd_mask = nvdimm_cmd_mask(nvdimm);
   480                  dsm_mask = nfit_mem->dsm_mask;
   481                  desc = nd_cmd_dimm_desc(cmd);
   482                  guid = to_nfit_uuid(nfit_mem->family);
   483                  handle = adev->handle;
   484          } else {
   485                  struct acpi_device *adev = to_acpi_dev(acpi_desc);
   486  
   487                  cmd_name = nvdimm_bus_cmd_name(cmd);
   488                  cmd_mask = nd_desc->cmd_mask;
   489                  dsm_mask = nd_desc->bus_dsm_mask;
   490                  desc = nd_cmd_bus_desc(cmd);
   491                  guid = to_nfit_uuid(NFIT_DEV_BUS);
   492                  handle = adev->handle;
   493                  dimm_name = "bus";
   494          }
   495  
   496          if (!desc || (cmd && (desc->out_num + desc->in_num == 0)))
   497                  return -ENOTTY;
   498  
   499          /*
   500           * Check for a valid command.  For ND_CMD_CALL, we also have to
   501           * make sure that the DSM function is supported.
   502           */
   503          if (cmd == ND_CMD_CALL && !test_bit(func, &dsm_mask))
                                                    ^^^^

"func" might be beyond the end of the bitmap so it could be an out of bounds read. We do bounds check it to <= 31 in nfit_dsm_revid() but I couldn't see any range checking in acpi_evaluate_dsm().

   504                  return -ENOTTY;
   505          else if (!test_bit(cmd, &cmd_mask))
   506                  return -ENOTTY;
   507  
   508          in_obj.type = ACPI_TYPE_PACKAGE;
   509          in_obj.package.count = 1;
   510          in_obj.package.elements = &in_buf;
   511          in_buf.type = ACPI_TYPE_BUFFER;
   512          in_buf.buffer.pointer = buf;
   513          in_buf.buffer.length = 0;
   514  
   515          /* libnvdimm has already validated the input envelope */
   516          for (i = 0; i < desc->in_num; i++)
   517                  in_buf.buffer.length += nd_cmd_in_size(nvdimm, cmd, desc,
   518                                  i, buf);
   519  
   520          if (call_pkg) {
   521                  /* skip over package wrapper */
   522                  in_buf.buffer.pointer = (void *) &call_pkg->nd_payload;
   523                  in_buf.buffer.length = call_pkg->nd_size_in;
   524          }
   525  
   526          dev_dbg(dev, "%s cmd: %d: func: %d input length: %d\n",
   527                  dimm_name, cmd, func, in_buf.buffer.length);
   528          if (payload_dumpable(nvdimm, func))

[ snip ]

3500 /* prevent security commands from being issued via ioctl */ 3501 static int acpi_nfit_clear_to_send(struct nvdimm_bus_descriptor *nd_desc,
3502 struct nvdimm *nvdimm, unsigned int cmd, void *buf)
3503 {
3504 struct nd_cmd_pkg *call_pkg = buf;
3505 unsigned int func;
3506
3507 if (nvdimm && cmd == ND_CMD_CALL &&
3508 call_pkg->nd_family == NVDIMM_FAMILY_INTEL) {
3509 func = call_pkg->nd_command;
3510 if ((1 << func) & NVDIMM_INTEL_SECURITY_CMDMASK)

                             ^^^^^^^^^

This is undefined and it would be nice to fix, but not important for run time.

  3511                          return -EOPNOTSUPP;
  3512          }
  3513  
  3514          return __acpi_nfit_clear_to_send(nd_desc, nvdimm, cmd);
  3515  }

regards,
dan carpenter