* [bug report] acpi/nfit: Add support for Intel DSM 1.8 commands
@ 2019-05-02 10:07 Dan Carpenter
2019-05-02 10:07 ` Dan Carpenter
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2019-05-02 10:07 UTC (permalink / raw)
To: Dan Williams, dave.jiang-ral2JQCrhuEAvxtiuMwx3w
Cc: linux-acpi-u79uwXL29TY76Z2rM5mHXA, linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw
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
^ permalink raw reply [flat|nested] 2+ messages in thread
* [bug report] acpi/nfit: Add support for Intel DSM 1.8 commands
2019-05-02 10:07 [bug report] acpi/nfit: Add support for Intel DSM 1.8 commands Dan Carpenter
@ 2019-05-02 10:07 ` Dan Carpenter
0 siblings, 0 replies; 2+ messages in thread
From: Dan Carpenter @ 2019-05-02 10:07 UTC (permalink / raw)
To: Dan Williams, dave.jiang; +Cc: linux-nvdimm, linux-acpi
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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-05-02 10:07 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-02 10:07 [bug report] acpi/nfit: Add support for Intel DSM 1.8 commands Dan Carpenter
2019-05-02 10:07 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).