diff mbox series

libnvdimm/label: Clear 'updating' flag after label-set update

Message ID 154758319127.1865528.486579211628557163.stgit@dwillia2-desk3.amr.corp.intel.com (mailing list archive)
State Mainlined
Commit 966d23a006ca7b44ac8cf4d0c96b19785e0c3da0
Headers show
Series libnvdimm/label: Clear 'updating' flag after label-set update | expand

Commit Message

Dan Williams Jan. 15, 2019, 8:13 p.m. UTC 
The UEFI 2.7 specification sets expectations that the 'updating' flag is
eventually cleared. To date, the libnvdimm core has never adhered to
that protocol. The policy of the core matches the policy of other
multi-device info-block formats like MD-Software-RAID that expect
administrator intervention on inconsistent info-blocks, not automatic
invalidation.

However, some pre-boot environments may unfortunately attempt to "clean
up" the labels and invalidate a set when it fails to find at least one
"non-updating" label in the set. Clear the updating flag after set
updates to minimize the window of vulnerability to aggressive pre-boot
environments.

Ideally implementations would not write to the label area outside of
creating namespaces.

Note that this only minimizes the window, it does not close it as the
system can still crash while clearing the flag and the set can be
subsequently deleted / invalidated by the pre-boot environment.

Fixes: f524bf271a5c ("libnvdimm: write pmem label set")
Cc: <stable@vger.kernel.org>
Cc: Kelly Couch <kelly.j.couch@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/nvdimm/label.c |   23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

Comments

Sasha Levin Jan. 18, 2019, 3:01 p.m. UTC | #1 
Hi,

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag,
fixing commit: f524bf271a5c libnvdimm: write pmem label set.

The bot has tested the following trees: v4.20.2, v4.19.15, v4.14.93, v4.9.150, v4.4.170.

v4.20.2: Build OK!
v4.19.15: Build OK!
v4.14.93: Build failed! Errors:

v4.9.150: Build OK!
v4.4.170: Failed to apply! Possible dependencies:
    0caeef63e6d2 ("libnvdimm: Add a poison list and export badblocks")
    16660eaea0cc ("libnvdimm, namespace: update label implementation for multi-pmem")
    1cf03c00e7c1 ("nfit: scrub and register regions in a workqueue")
    31eca76ba2fc ("nfit, libnvdimm: limited/whitelisted dimm command marshaling mechanism")
    44c462eb9e19 ("libnvdimm, region: move region-mapping input-paramters to nd_mapping_desc")
    4577b0665515 ("nfit: update address range scrub commands to the acpi 6.1 format")
    6e2452dff444 ("nfit: Continue init even if ARS commands are unimplemented")
    747ffe11b440 ("libnvdimm, tools/testing/nvdimm: fix 'ars_status' output buffer sizing")
    7ae0fa439faf ("nfit, libnvdimm: async region scrub workqueue")
    a61fe6f7902e ("nfit, tools/testing/nvdimm: unify common init for acpi_nfit_desc")
    ad9ac5e19575 ("nfit: always associate flush hints")
    ae8219f186d8 ("libnvdimm, label: convert label tracking to a linked list")
    aef253382266 ("libnvdimm, nfit: centralize command status translation")
    d26f73f083ed ("nfit_test: Enable DSMs for all test NFITs")
    e3654eca70d6 ("nfit, libnvdimm: clarify "commands" vs "_DSMs"")
    e5ae3b252c67 ("libnvdimm, nfit: move flush hint mapping to region-device driver-data")


How should we proceed with this patch?

--
Thanks,
Sasha
Sasha Levin Jan. 18, 2019, 3:01 p.m. UTC | #2 
Hi,

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag,
fixing commit: f524bf271a5c libnvdimm: write pmem label set.

The bot has tested the following trees: v4.20.2, v4.19.15, v4.14.93, v4.9.150, v4.4.170.

v4.20.2: Build OK!
v4.19.15: Build OK!
v4.14.93: Build OK!
v4.9.150: Build OK!
v4.4.170: Failed to apply! Possible dependencies:
    0caeef63e6d2 ("libnvdimm: Add a poison list and export badblocks")
    16660eaea0cc ("libnvdimm, namespace: update label implementation for multi-pmem")
    1cf03c00e7c1 ("nfit: scrub and register regions in a workqueue")
    31eca76ba2fc ("nfit, libnvdimm: limited/whitelisted dimm command marshaling mechanism")
    44c462eb9e19 ("libnvdimm, region: move region-mapping input-paramters to nd_mapping_desc")
    4577b0665515 ("nfit: update address range scrub commands to the acpi 6.1 format")
    6e2452dff444 ("nfit: Continue init even if ARS commands are unimplemented")
    747ffe11b440 ("libnvdimm, tools/testing/nvdimm: fix 'ars_status' output buffer sizing")
    7ae0fa439faf ("nfit, libnvdimm: async region scrub workqueue")
    a61fe6f7902e ("nfit, tools/testing/nvdimm: unify common init for acpi_nfit_desc")
    ad9ac5e19575 ("nfit: always associate flush hints")
    ae8219f186d8 ("libnvdimm, label: convert label tracking to a linked list")
    aef253382266 ("libnvdimm, nfit: centralize command status translation")
    d26f73f083ed ("nfit_test: Enable DSMs for all test NFITs")
    e3654eca70d6 ("nfit, libnvdimm: clarify "commands" vs "_DSMs"")
    e5ae3b252c67 ("libnvdimm, nfit: move flush hint mapping to region-device driver-data")


How should we proceed with this patch?

--
Thanks,
Sasha
diff mbox series

Patch

diff --git a/drivers/nvdimm/label.c b/drivers/nvdimm/label.c
index a11bf4e6b451..6d6e9a12150b 100644
--- a/drivers/nvdimm/label.c
+++ b/drivers/nvdimm/label.c
@@ -755,7 +755,7 @@  static const guid_t *to_abstraction_guid(enum nvdimm_claim_class claim_class,
 
 static int __pmem_label_update(struct nd_region *nd_region,
 		struct nd_mapping *nd_mapping, struct nd_namespace_pmem *nspm,
-		int pos)
+		int pos, unsigned long flags)
 {
 	struct nd_namespace_common *ndns = &nspm->nsio.common;
 	struct nd_interleave_set *nd_set = nd_region->nd_set;
@@ -796,7 +796,7 @@  static int __pmem_label_update(struct nd_region *nd_region,
 	memcpy(nd_label->uuid, nspm->uuid, NSLABEL_UUID_LEN);
 	if (nspm->alt_name)
 		memcpy(nd_label->name, nspm->alt_name, NSLABEL_NAME_LEN);
-	nd_label->flags = __cpu_to_le32(NSLABEL_FLAG_UPDATING);
+	nd_label->flags = __cpu_to_le32(flags);
 	nd_label->nlabel = __cpu_to_le16(nd_region->ndr_mappings);
 	nd_label->position = __cpu_to_le16(pos);
 	nd_label->isetcookie = __cpu_to_le64(cookie);
@@ -1249,13 +1249,13 @@  static int del_labels(struct nd_mapping *nd_mapping, u8 *uuid)
 int nd_pmem_namespace_label_update(struct nd_region *nd_region,
 		struct nd_namespace_pmem *nspm, resource_size_t size)
 {
-	int i;
+	int i, rc;
 
 	for (i = 0; i < nd_region->ndr_mappings; i++) {
 		struct nd_mapping *nd_mapping = &nd_region->mapping[i];
 		struct nvdimm_drvdata *ndd = to_ndd(nd_mapping);
 		struct resource *res;
-		int rc, count = 0;
+		int count = 0;
 
 		if (size == 0) {
 			rc = del_labels(nd_mapping, nspm->uuid);
@@ -1273,7 +1273,20 @@  int nd_pmem_namespace_label_update(struct nd_region *nd_region,
 		if (rc < 0)
 			return rc;
 
-		rc = __pmem_label_update(nd_region, nd_mapping, nspm, i);
+		rc = __pmem_label_update(nd_region, nd_mapping, nspm, i,
+				NSLABEL_FLAG_UPDATING);
+		if (rc)
+			return rc;
+	}
+
+	if (size == 0)
+		return 0;
+
+	/* Clear the UPDATING flag per UEFI 2.7 expectations */
+	for (i = 0; i < nd_region->ndr_mappings; i++) {
+		struct nd_mapping *nd_mapping = &nd_region->mapping[i];
+
+		rc = __pmem_label_update(nd_region, nd_mapping, nspm, i, 0);
 		if (rc)
 			return rc;
 	}