* [bug report] acpi/nfit: fix cmd_rc for acpi_nfit_ctl to always return a value
@ 2018-07-11 9:07 Dan Carpenter
0 siblings, 0 replies; only message in thread
From: Dan Carpenter @ 2018-07-11 9:07 UTC (permalink / raw)
To: dave.jiang; +Cc: linux-nvdimm
Hello Dave Jiang,
This is a semi-automatic email about new static checker warnings.
The patch c1985cefd844: "acpi/nfit: fix cmd_rc for acpi_nfit_ctl to
always return a value" from Jun 28, 2018, leads to the following
Smatch complaint:
drivers/acpi/nfit/core.c:578 acpi_nfit_ctl()
warn: variable dereferenced before check 'cmd_rc' (see line 411)
drivers/acpi/nfit/core.c
410
411 *cmd_rc = -EINVAL;
^^^^^^^^^^^^^^^^^^
Patch adds unchecked dereference.
412 func = cmd;
413 if (cmd == ND_CMD_CALL) {
414 call_pkg = buf;
415 func = call_pkg->nd_command;
416
417 for (i = 0; i < ARRAY_SIZE(call_pkg->nd_reserved2); i++)
418 if (call_pkg->nd_reserved2[i])
419 return -EINVAL;
420 }
421
422 if (nvdimm) {
423 struct acpi_device *adev = nfit_mem->adev;
424
425 if (!adev)
426 return -ENOTTY;
427 if (call_pkg && nfit_mem->family != call_pkg->nd_family)
428 return -ENOTTY;
429
430 dimm_name = nvdimm_name(nvdimm);
431 cmd_name = nvdimm_cmd_name(cmd);
432 cmd_mask = nvdimm_cmd_mask(nvdimm);
433 dsm_mask = nfit_mem->dsm_mask;
434 desc = nd_cmd_dimm_desc(cmd);
435 guid = to_nfit_uuid(nfit_mem->family);
436 handle = adev->handle;
437 } else {
438 struct acpi_device *adev = to_acpi_dev(acpi_desc);
439
440 cmd_name = nvdimm_bus_cmd_name(cmd);
441 cmd_mask = nd_desc->cmd_mask;
442 dsm_mask = cmd_mask;
443 if (cmd == ND_CMD_CALL)
444 dsm_mask = nd_desc->bus_dsm_mask;
445 desc = nd_cmd_bus_desc(cmd);
446 guid = to_nfit_uuid(NFIT_DEV_BUS);
447 handle = adev->handle;
448 dimm_name = "bus";
449 }
450
451 if (!desc || (cmd && (desc->out_num + desc->in_num == 0)))
452 return -ENOTTY;
453
454 if (!test_bit(cmd, &cmd_mask) || !test_bit(func, &dsm_mask))
455 return -ENOTTY;
456
457 in_obj.type = ACPI_TYPE_PACKAGE;
458 in_obj.package.count = 1;
459 in_obj.package.elements = &in_buf;
460 in_buf.type = ACPI_TYPE_BUFFER;
461 in_buf.buffer.pointer = buf;
462 in_buf.buffer.length = 0;
463
464 /* libnvdimm has already validated the input envelope */
465 for (i = 0; i < desc->in_num; i++)
466 in_buf.buffer.length += nd_cmd_in_size(nvdimm, cmd, desc,
467 i, buf);
468
469 if (call_pkg) {
470 /* skip over package wrapper */
471 in_buf.buffer.pointer = (void *) &call_pkg->nd_payload;
472 in_buf.buffer.length = call_pkg->nd_size_in;
473 }
474
475 dev_dbg(dev, "%s cmd: %d: func: %d input length: %d\n",
476 dimm_name, cmd, func, in_buf.buffer.length);
477 print_hex_dump_debug("nvdimm in ", DUMP_PREFIX_OFFSET, 4, 4,
478 in_buf.buffer.pointer,
479 min_t(u32, 256, in_buf.buffer.length), true);
480
481 /* call the BIOS, prefer the named methods over _DSM if available */
482 if (nvdimm && cmd == ND_CMD_GET_CONFIG_SIZE && nfit_mem->has_lsr)
483 out_obj = acpi_label_info(handle);
484 else if (nvdimm && cmd == ND_CMD_GET_CONFIG_DATA && nfit_mem->has_lsr) {
485 struct nd_cmd_get_config_data_hdr *p = buf;
486
487 out_obj = acpi_label_read(handle, p->in_offset, p->in_length);
488 } else if (nvdimm && cmd == ND_CMD_SET_CONFIG_DATA
489 && nfit_mem->has_lsw) {
490 struct nd_cmd_set_config_hdr *p = buf;
491
492 out_obj = acpi_label_write(handle, p->in_offset, p->in_length,
493 p->in_buf);
494 } else {
495 u8 revid;
496
497 if (nvdimm)
498 revid = nfit_dsm_revid(nfit_mem->family, func);
499 else
500 revid = 1;
501 out_obj = acpi_evaluate_dsm(handle, guid, revid, func, &in_obj);
502 }
503
504 if (!out_obj) {
505 dev_dbg(dev, "%s _DSM failed cmd: %s\n", dimm_name, cmd_name);
506 return -EINVAL;
507 }
508
509 if (call_pkg) {
510 call_pkg->nd_fw_size = out_obj->buffer.length;
511 memcpy(call_pkg->nd_payload + call_pkg->nd_size_in,
512 out_obj->buffer.pointer,
513 min(call_pkg->nd_fw_size, call_pkg->nd_size_out));
514
515 ACPI_FREE(out_obj);
516 /*
517 * Need to support FW function w/o known size in advance.
518 * Caller can determine required size based upon nd_fw_size.
519 * If we return an error (like elsewhere) then caller wouldn't
520 * be able to rely upon data returned to make calculation.
521 */
522 *cmd_rc = 0;
523 return 0;
524 }
525
526 if (out_obj->package.type != ACPI_TYPE_BUFFER) {
527 dev_dbg(dev, "%s unexpected output object type cmd: %s type: %d\n",
528 dimm_name, cmd_name, out_obj->type);
529 rc = -EINVAL;
530 goto out;
531 }
532
533 dev_dbg(dev, "%s cmd: %s output length: %d\n", dimm_name,
534 cmd_name, out_obj->buffer.length);
535 print_hex_dump_debug(cmd_name, DUMP_PREFIX_OFFSET, 4, 4,
536 out_obj->buffer.pointer,
537 min_t(u32, 128, out_obj->buffer.length), true);
538
539 for (i = 0, offset = 0; i < desc->out_num; i++) {
540 u32 out_size = nd_cmd_out_size(nvdimm, cmd, desc, i, buf,
541 (u32 *) out_obj->buffer.pointer,
542 out_obj->buffer.length - offset);
543
544 if (offset + out_size > out_obj->buffer.length) {
545 dev_dbg(dev, "%s output object underflow cmd: %s field: %d\n",
546 dimm_name, cmd_name, i);
547 break;
548 }
549
550 if (in_buf.buffer.length + offset + out_size > buf_len) {
551 dev_dbg(dev, "%s output overrun cmd: %s field: %d\n",
552 dimm_name, cmd_name, i);
553 rc = -ENXIO;
554 goto out;
555 }
556 memcpy(buf + in_buf.buffer.length + offset,
557 out_obj->buffer.pointer + offset, out_size);
558 offset += out_size;
559 }
560
561 /*
562 * Set fw_status for all the commands with a known format to be
563 * later interpreted by xlat_status().
564 */
565 if (i >= 1 && ((!nvdimm && cmd >= ND_CMD_ARS_CAP
566 && cmd <= ND_CMD_CLEAR_ERROR)
567 || (nvdimm && cmd >= ND_CMD_SMART
568 && cmd <= ND_CMD_VENDOR)))
569 fw_status = *(u32 *) out_obj->buffer.pointer;
570
571 if (offset + in_buf.buffer.length < buf_len) {
572 if (i >= 1) {
573 /*
574 * status valid, return the number of bytes left
575 * unfilled in the output buffer
576 */
577 rc = buf_len - offset - in_buf.buffer.length;
578 if (cmd_rc)
^^^^^^
Checked too late.
579 *cmd_rc = xlat_status(nvdimm, buf, cmd,
580 fw_status);
regards,
dan carpenter
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2018-07-11 9:08 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-11 9:07 [bug report] acpi/nfit: fix cmd_rc for acpi_nfit_ctl to always return a value 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).