Skip to content

Commit 810ac7b

Browse files
committed
Merge branch 'libnvdimm-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm
Pull libnvdimm fixes from Dan Williams: "Several fixes to the DSM (ACPI device specific method) marshaling implementation. I consider these urgent enough to send for 4.9 consideration since they fix the kernel's handling of ARS (Address Range Scrub) commands. Especially for platforms without machine-check-recovery capabilities, successful execution of ARS commands enables the platform to potentially break out of an infinite reboot problem if a media error is present in the boot path. There is also a one line fix for a device-dax read-only mapping regression. Commits 9a901f5 ("acpi, nfit: fix extended status translations for ACPI DSMs") and 325896f ("device-dax: fix private mapping restriction, permit read-only") are true regression fixes for changes introduced this cycle. Commit efda1b5 ("acpi, nfit, libnvdimm: fix / harden ars_status output length handling") fixes the kernel's handling of zero-length results, this never would have worked in the past, but we only just recently discovered a BIOS implementation that emits this arguably spec non-compliant result. The remaining two commits are additional fall out from thinking through the implications of a zero / truncated length result of the ARS Status command. In order to mitigate the risk that these changes introduce yet more regressions they are backstopped by a new unit test in commit a7de92d ("tools/testing/nvdimm: unit test acpi_nfit_ctl()") that mocks up inputs to acpi_nfit_ctl()" * 'libnvdimm-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm: device-dax: fix private mapping restriction, permit read-only tools/testing/nvdimm: unit test acpi_nfit_ctl() acpi, nfit: fix bus vs dimm confusion in xlat_status acpi, nfit: validate ars_status output buffer size acpi, nfit, libnvdimm: fix / harden ars_status output length handling acpi, nfit: fix extended status translations for ACPI DSMs
2 parents 861d75d + 325896f commit 810ac7b

File tree

9 files changed

+326
-28
lines changed

9 files changed

+326
-28
lines changed

drivers/acpi/nfit/core.c

Lines changed: 40 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ static struct acpi_device *to_acpi_dev(struct acpi_nfit_desc *acpi_desc)
9494
return to_acpi_device(acpi_desc->dev);
9595
}
9696

97-
static int xlat_status(void *buf, unsigned int cmd, u32 status)
97+
static int xlat_bus_status(void *buf, unsigned int cmd, u32 status)
9898
{
9999
struct nd_cmd_clear_error *clear_err;
100100
struct nd_cmd_ars_status *ars_status;
@@ -113,7 +113,7 @@ static int xlat_status(void *buf, unsigned int cmd, u32 status)
113113
flags = ND_ARS_PERSISTENT | ND_ARS_VOLATILE;
114114
if ((status >> 16 & flags) == 0)
115115
return -ENOTTY;
116-
break;
116+
return 0;
117117
case ND_CMD_ARS_START:
118118
/* ARS is in progress */
119119
if ((status & 0xffff) == NFIT_ARS_START_BUSY)
@@ -122,7 +122,7 @@ static int xlat_status(void *buf, unsigned int cmd, u32 status)
122122
/* Command failed */
123123
if (status & 0xffff)
124124
return -EIO;
125-
break;
125+
return 0;
126126
case ND_CMD_ARS_STATUS:
127127
ars_status = buf;
128128
/* Command failed */
@@ -146,15 +146,16 @@ static int xlat_status(void *buf, unsigned int cmd, u32 status)
146146
* then just continue with the returned results.
147147
*/
148148
if (status == NFIT_ARS_STATUS_INTR) {
149-
if (ars_status->flags & NFIT_ARS_F_OVERFLOW)
149+
if (ars_status->out_length >= 40 && (ars_status->flags
150+
& NFIT_ARS_F_OVERFLOW))
150151
return -ENOSPC;
151152
return 0;
152153
}
153154

154155
/* Unknown status */
155156
if (status >> 16)
156157
return -EIO;
157-
break;
158+
return 0;
158159
case ND_CMD_CLEAR_ERROR:
159160
clear_err = buf;
160161
if (status & 0xffff)
@@ -163,7 +164,7 @@ static int xlat_status(void *buf, unsigned int cmd, u32 status)
163164
return -EIO;
164165
if (clear_err->length > clear_err->cleared)
165166
return clear_err->cleared;
166-
break;
167+
return 0;
167168
default:
168169
break;
169170
}
@@ -174,9 +175,18 @@ static int xlat_status(void *buf, unsigned int cmd, u32 status)
174175
return 0;
175176
}
176177

177-
static int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
178-
struct nvdimm *nvdimm, unsigned int cmd, void *buf,
179-
unsigned int buf_len, int *cmd_rc)
178+
static int xlat_status(struct nvdimm *nvdimm, void *buf, unsigned int cmd,
179+
u32 status)
180+
{
181+
if (!nvdimm)
182+
return xlat_bus_status(buf, cmd, status);
183+
if (status)
184+
return -EIO;
185+
return 0;
186+
}
187+
188+
int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm,
189+
unsigned int cmd, void *buf, unsigned int buf_len, int *cmd_rc)
180190
{
181191
struct acpi_nfit_desc *acpi_desc = to_acpi_nfit_desc(nd_desc);
182192
union acpi_object in_obj, in_buf, *out_obj;
@@ -298,7 +308,8 @@ static int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
298308

299309
for (i = 0, offset = 0; i < desc->out_num; i++) {
300310
u32 out_size = nd_cmd_out_size(nvdimm, cmd, desc, i, buf,
301-
(u32 *) out_obj->buffer.pointer);
311+
(u32 *) out_obj->buffer.pointer,
312+
out_obj->buffer.length - offset);
302313

303314
if (offset + out_size > out_obj->buffer.length) {
304315
dev_dbg(dev, "%s:%s output object underflow cmd: %s field: %d\n",
@@ -333,7 +344,8 @@ static int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
333344
*/
334345
rc = buf_len - offset - in_buf.buffer.length;
335346
if (cmd_rc)
336-
*cmd_rc = xlat_status(buf, cmd, fw_status);
347+
*cmd_rc = xlat_status(nvdimm, buf, cmd,
348+
fw_status);
337349
} else {
338350
dev_err(dev, "%s:%s underrun cmd: %s buf_len: %d out_len: %d\n",
339351
__func__, dimm_name, cmd_name, buf_len,
@@ -343,14 +355,15 @@ static int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
343355
} else {
344356
rc = 0;
345357
if (cmd_rc)
346-
*cmd_rc = xlat_status(buf, cmd, fw_status);
358+
*cmd_rc = xlat_status(nvdimm, buf, cmd, fw_status);
347359
}
348360

349361
out:
350362
ACPI_FREE(out_obj);
351363

352364
return rc;
353365
}
366+
EXPORT_SYMBOL_GPL(acpi_nfit_ctl);
354367

355368
static const char *spa_type_name(u16 type)
356369
{
@@ -2001,19 +2014,32 @@ static int ars_get_status(struct acpi_nfit_desc *acpi_desc)
20012014
return cmd_rc;
20022015
}
20032016

2004-
static int ars_status_process_records(struct nvdimm_bus *nvdimm_bus,
2017+
static int ars_status_process_records(struct acpi_nfit_desc *acpi_desc,
20052018
struct nd_cmd_ars_status *ars_status)
20062019
{
2020+
struct nvdimm_bus *nvdimm_bus = acpi_desc->nvdimm_bus;
20072021
int rc;
20082022
u32 i;
20092023

2024+
/*
2025+
* First record starts at 44 byte offset from the start of the
2026+
* payload.
2027+
*/
2028+
if (ars_status->out_length < 44)
2029+
return 0;
20102030
for (i = 0; i < ars_status->num_records; i++) {
2031+
/* only process full records */
2032+
if (ars_status->out_length
2033+
< 44 + sizeof(struct nd_ars_record) * (i + 1))
2034+
break;
20112035
rc = nvdimm_bus_add_poison(nvdimm_bus,
20122036
ars_status->records[i].err_address,
20132037
ars_status->records[i].length);
20142038
if (rc)
20152039
return rc;
20162040
}
2041+
if (i < ars_status->num_records)
2042+
dev_warn(acpi_desc->dev, "detected truncated ars results\n");
20172043

20182044
return 0;
20192045
}
@@ -2266,8 +2292,7 @@ static int acpi_nfit_query_poison(struct acpi_nfit_desc *acpi_desc,
22662292
if (rc < 0 && rc != -ENOSPC)
22672293
return rc;
22682294

2269-
if (ars_status_process_records(acpi_desc->nvdimm_bus,
2270-
acpi_desc->ars_status))
2295+
if (ars_status_process_records(acpi_desc, acpi_desc->ars_status))
22712296
return -ENOMEM;
22722297

22732298
return 0;

drivers/acpi/nfit/nfit.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,5 +240,7 @@ const u8 *to_nfit_uuid(enum nfit_uuids id);
240240
int acpi_nfit_init(struct acpi_nfit_desc *acpi_desc, void *nfit, acpi_size sz);
241241
void __acpi_nfit_notify(struct device *dev, acpi_handle handle, u32 event);
242242
void __acpi_nvdimm_notify(struct device *dev, u32 event);
243+
int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm,
244+
unsigned int cmd, void *buf, unsigned int buf_len, int *cmd_rc);
243245
void acpi_nfit_desc_init(struct acpi_nfit_desc *acpi_desc, struct device *dev);
244246
#endif /* __NFIT_H__ */

drivers/dax/dax.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,7 @@ static int check_vma(struct dax_dev *dax_dev, struct vm_area_struct *vma,
271271
return -ENXIO;
272272

273273
/* prevent private mappings from being established */
274-
if ((vma->vm_flags & VM_SHARED) != VM_SHARED) {
274+
if ((vma->vm_flags & VM_MAYSHARE) != VM_MAYSHARE) {
275275
dev_info(dev, "%s: %s: fail, attempted private mapping\n",
276276
current->comm, func);
277277
return -EINVAL;

drivers/nvdimm/bus.c

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -715,7 +715,7 @@ EXPORT_SYMBOL_GPL(nd_cmd_in_size);
715715

716716
u32 nd_cmd_out_size(struct nvdimm *nvdimm, int cmd,
717717
const struct nd_cmd_desc *desc, int idx, const u32 *in_field,
718-
const u32 *out_field)
718+
const u32 *out_field, unsigned long remainder)
719719
{
720720
if (idx >= desc->out_num)
721721
return UINT_MAX;
@@ -727,9 +727,24 @@ u32 nd_cmd_out_size(struct nvdimm *nvdimm, int cmd,
727727
return in_field[1];
728728
else if (nvdimm && cmd == ND_CMD_VENDOR && idx == 2)
729729
return out_field[1];
730-
else if (!nvdimm && cmd == ND_CMD_ARS_STATUS && idx == 2)
731-
return out_field[1] - 8;
732-
else if (cmd == ND_CMD_CALL) {
730+
else if (!nvdimm && cmd == ND_CMD_ARS_STATUS && idx == 2) {
731+
/*
732+
* Per table 9-276 ARS Data in ACPI 6.1, out_field[1] is
733+
* "Size of Output Buffer in bytes, including this
734+
* field."
735+
*/
736+
if (out_field[1] < 4)
737+
return 0;
738+
/*
739+
* ACPI 6.1 is ambiguous if 'status' is included in the
740+
* output size. If we encounter an output size that
741+
* overshoots the remainder by 4 bytes, assume it was
742+
* including 'status'.
743+
*/
744+
if (out_field[1] - 8 == remainder)
745+
return remainder;
746+
return out_field[1] - 4;
747+
} else if (cmd == ND_CMD_CALL) {
733748
struct nd_cmd_pkg *pkg = (struct nd_cmd_pkg *) in_field;
734749

735750
return pkg->nd_size_out;
@@ -876,7 +891,7 @@ static int __nd_ioctl(struct nvdimm_bus *nvdimm_bus, struct nvdimm *nvdimm,
876891
/* process an output envelope */
877892
for (i = 0; i < desc->out_num; i++) {
878893
u32 out_size = nd_cmd_out_size(nvdimm, cmd, desc, i,
879-
(u32 *) in_env, (u32 *) out_env);
894+
(u32 *) in_env, (u32 *) out_env, 0);
880895
u32 copy;
881896

882897
if (out_size == UINT_MAX) {

include/linux/libnvdimm.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ u32 nd_cmd_in_size(struct nvdimm *nvdimm, int cmd,
143143
const struct nd_cmd_desc *desc, int idx, void *buf);
144144
u32 nd_cmd_out_size(struct nvdimm *nvdimm, int cmd,
145145
const struct nd_cmd_desc *desc, int idx, const u32 *in_field,
146-
const u32 *out_field);
146+
const u32 *out_field, unsigned long remainder);
147147
int nvdimm_bus_check_dimm_count(struct nvdimm_bus *nvdimm_bus, int dimm_count);
148148
struct nd_region *nvdimm_pmem_region_create(struct nvdimm_bus *nvdimm_bus,
149149
struct nd_region_desc *ndr_desc);

tools/testing/nvdimm/Kbuild

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ ldflags-y += --wrap=devm_memremap_pages
1414
ldflags-y += --wrap=insert_resource
1515
ldflags-y += --wrap=remove_resource
1616
ldflags-y += --wrap=acpi_evaluate_object
17+
ldflags-y += --wrap=acpi_evaluate_dsm
1718

1819
DRIVERS := ../../../drivers
1920
NVDIMM_SRC := $(DRIVERS)/nvdimm

tools/testing/nvdimm/test/iomap.c

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,17 @@ static LIST_HEAD(iomap_head);
2626

2727
static struct iomap_ops {
2828
nfit_test_lookup_fn nfit_test_lookup;
29+
nfit_test_evaluate_dsm_fn evaluate_dsm;
2930
struct list_head list;
3031
} iomap_ops = {
3132
.list = LIST_HEAD_INIT(iomap_ops.list),
3233
};
3334

34-
void nfit_test_setup(nfit_test_lookup_fn lookup)
35+
void nfit_test_setup(nfit_test_lookup_fn lookup,
36+
nfit_test_evaluate_dsm_fn evaluate)
3537
{
3638
iomap_ops.nfit_test_lookup = lookup;
39+
iomap_ops.evaluate_dsm = evaluate;
3740
list_add_rcu(&iomap_ops.list, &iomap_head);
3841
}
3942
EXPORT_SYMBOL(nfit_test_setup);
@@ -367,4 +370,22 @@ acpi_status __wrap_acpi_evaluate_object(acpi_handle handle, acpi_string path,
367370
}
368371
EXPORT_SYMBOL(__wrap_acpi_evaluate_object);
369372

373+
union acpi_object * __wrap_acpi_evaluate_dsm(acpi_handle handle, const u8 *uuid,
374+
u64 rev, u64 func, union acpi_object *argv4)
375+
{
376+
union acpi_object *obj = ERR_PTR(-ENXIO);
377+
struct iomap_ops *ops;
378+
379+
rcu_read_lock();
380+
ops = list_first_or_null_rcu(&iomap_head, typeof(*ops), list);
381+
if (ops)
382+
obj = ops->evaluate_dsm(handle, uuid, rev, func, argv4);
383+
rcu_read_unlock();
384+
385+
if (IS_ERR(obj))
386+
return acpi_evaluate_dsm(handle, uuid, rev, func, argv4);
387+
return obj;
388+
}
389+
EXPORT_SYMBOL(__wrap_acpi_evaluate_dsm);
390+
370391
MODULE_LICENSE("GPL v2");

0 commit comments

Comments
 (0)