queue-5.0/nfit-fix-nfit_intel_shutdown_status-command-submission.patch

   1 From f596c8844fe1d0022007ae6c7a377361fb653eff Mon Sep 17 00:00:00 2001
   2 From: Dan Williams <dan.j.williams@intel.com>
   3 Date: Tue, 29 Jan 2019 22:06:41 -0800
   4 Subject: nfit: Fix nfit_intel_shutdown_status() command submission
   5
   6 From: Dan Williams <dan.j.williams@intel.com>
   7
   8 commit f596c8844fe1d0022007ae6c7a377361fb653eff upstream.
   9
  10 The implementation is broken in all the ways the unit test did not touch:
  11
  12 1/ The local definition of in_buf and in_obj violated C99 initializer
  13    expectations for zeroing. By only initializing 2 out of the three
  14    struct members the compiler was free to zero-initialize the remaining
  15    entry even though the aliased location in the union was initialized.
  16
  17 2/ The implementation made assumptions about the state of the 'smart'
  18    payload after command execution that are satisfied by
  19    acpi_nfit_ctl(), but not acpi_evaluate_dsm().
  20
  21 3/ populate_shutdown_status() is skipped on Intel NVDIMMs due to the early
  22    return for skipping the common _LS{I,R,W} enabling.
  23
  24 4/ The input length should be zero.
  25
  26 This breakage was missed due to the unit test implementation only
  27 testing the case where nfit_intel_shutdown_status() returns a valid
  28 payload.
  29
  30 Much of this complexity would be saved if acpi_nfit_ctl() could be used, but
  31 that currently requires a 'struct nvdimm *' argument and one is not created
  32 until later in the init process. The health result is needed before the device
  33 is created because the payload gates whether the nmemX/nfit/dirty_shutdown
  34 property is visible in sysfs.
  35
  36 Cc: <stable@vger.kernel.org>
  37 Fixes: 0ead11181fe0 ("acpi, nfit: Collect shutdown status")
  38 Reported-by: Dexuan Cui <decui@microsoft.com>
  39 Reviewed-by: Dexuan Cui <decui@microsoft.com>
  40 Signed-off-by: Dan Williams <dan.j.williams@intel.com>
  41 Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  42
  43 ---
  44  drivers/acpi/nfit/core.c |   43 +++++++++++++++++++++++++------------------
  45  1 file changed, 25 insertions(+), 18 deletions(-)
  46
  47 --- a/drivers/acpi/nfit/core.c
  48 +++ b/drivers/acpi/nfit/core.c
  49 @@ -1759,14 +1759,14 @@ static bool acpi_nvdimm_has_method(struc
  50
  51  __weak void nfit_intel_shutdown_status(struct nfit_mem *nfit_mem)
  52  {
  53 +       struct device *dev = &nfit_mem->adev->dev;
  54         struct nd_intel_smart smart = { 0 };
  55         union acpi_object in_buf = {
  56 -               .type = ACPI_TYPE_BUFFER,
  57 -               .buffer.pointer = (char *) &smart,
  58 -               .buffer.length = sizeof(smart),
  59 +               .buffer.type = ACPI_TYPE_BUFFER,
  60 +               .buffer.length = 0,
  61         };
  62         union acpi_object in_obj = {
  63 -               .type = ACPI_TYPE_PACKAGE,
  64 +               .package.type = ACPI_TYPE_PACKAGE,
  65                 .package.count = 1,
  66                 .package.elements = &in_buf,
  67         };
  68 @@ -1781,8 +1781,15 @@ __weak void nfit_intel_shutdown_status(s
  69                 return;
  70
  71         out_obj = acpi_evaluate_dsm(handle, guid, revid, func, &in_obj);
  72 -       if (!out_obj)
  73 +       if (!out_obj || out_obj->type != ACPI_TYPE_BUFFER
  74 +                       || out_obj->buffer.length < sizeof(smart)) {
  75 +               dev_dbg(dev->parent, "%s: failed to retrieve initial health\n",
  76 +                               dev_name(dev));
  77 +               ACPI_FREE(out_obj);
  78                 return;
  79 +       }
  80 +       memcpy(&smart, out_obj->buffer.pointer, sizeof(smart));
  81 +       ACPI_FREE(out_obj);
  82
  83         if (smart.flags & ND_INTEL_SMART_SHUTDOWN_VALID) {
  84                 if (smart.shutdown_state)
  85 @@ -1793,7 +1800,6 @@ __weak void nfit_intel_shutdown_status(s
  86                 set_bit(NFIT_MEM_DIRTY_COUNT, &nfit_mem->flags);
  87                 nfit_mem->dirty_shutdown = smart.shutdown_count;
  88         }
  89 -       ACPI_FREE(out_obj);
  90  }
  91
  92  static void populate_shutdown_status(struct nfit_mem *nfit_mem)
  93 @@ -1915,18 +1921,19 @@ static int acpi_nfit_add_dimm(struct acp
  94                 | 1 << ND_CMD_SET_CONFIG_DATA;
  95         if (family == NVDIMM_FAMILY_INTEL
  96                         && (dsm_mask & label_mask) == label_mask)
  97 -               return 0;
  98 -
  99 -       if (acpi_nvdimm_has_method(adev_dimm, "_LSI")
 100 -                       && acpi_nvdimm_has_method(adev_dimm, "_LSR")) {
 101 -               dev_dbg(dev, "%s: has _LSR\n", dev_name(&adev_dimm->dev));
 102 -               set_bit(NFIT_MEM_LSR, &nfit_mem->flags);
 103 -       }
 104 -
 105 -       if (test_bit(NFIT_MEM_LSR, &nfit_mem->flags)
 106 -                       && acpi_nvdimm_has_method(adev_dimm, "_LSW")) {
 107 -               dev_dbg(dev, "%s: has _LSW\n", dev_name(&adev_dimm->dev));
 108 -               set_bit(NFIT_MEM_LSW, &nfit_mem->flags);
 109 +               /* skip _LS{I,R,W} enabling */;
 110 +       else {
 111 +               if (acpi_nvdimm_has_method(adev_dimm, "_LSI")
 112 +                               && acpi_nvdimm_has_method(adev_dimm, "_LSR")) {
 113 +                       dev_dbg(dev, "%s: has _LSR\n", dev_name(&adev_dimm->dev));
 114 +                       set_bit(NFIT_MEM_LSR, &nfit_mem->flags);
 115 +               }
 116 +
 117 +               if (test_bit(NFIT_MEM_LSR, &nfit_mem->flags)
 118 +                               && acpi_nvdimm_has_method(adev_dimm, "_LSW")) {
 119 +                       dev_dbg(dev, "%s: has _LSW\n", dev_name(&adev_dimm->dev));
 120 +                       set_bit(NFIT_MEM_LSW, &nfit_mem->flags);
 121 +               }
 122         }
 123
 124         populate_shutdown_status(nfit_mem);