Patch verified, cookies align again
On Wed, 2017-03-01 at 01:11 -0800, Dan Williams wrote:
> The interleave-set cookie is a sum that sanity checks the composition of
> an interleave set has not changed from when the namespace was initially
> created. The checksum is calculated by sorting the DIMMs by their
> location in the interleave-set. The comparison for the sort must be
> 64-bit wide, not byte-by-byte as performed by memcmp() in the broken
> case.
>
> Fix the implementation to accept correct cookie values in addition to
> the Linux "memcmp" order cookies, but only allow correct cookies to be
> generated going forward. It does mean that namespaces created by
> third-party-tooling, or created by newer kernels with this fix, will not
> validate on older kernels. However, there are a couple mitigating
> conditions:
>
> 1/ platforms with namespace-label capable NVDIMMs are not widely
> available.
>
> 2/ interleave-sets with a single-dimm are by definition not affected
> (nothing to sort). This covers the QEMU-KVM NVDIMM emulation case.
>
> The cookie stored in the namespace label can be fixed up by writing
> the namespace "alt_name" attribute in sysfs.
>
> Cc: <stable@xxxxxxxxxxxxxxx>
> Fixes: eaf961536e16 ("libnvdimm, nfit: add interleave-set state-tracking infrastructure")
> Reported-by: Nicholas Moulin <nicholas.w.moulin@xxxxxxxxxxxxxxx>
> Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
> ---
> drivers/acpi/nfit/core.c | 16 +++++++++++++++-
> drivers/nvdimm/namespace_devs.c | 18 ++++++++++++++----
> drivers/nvdimm/nd.h | 1 +
> drivers/nvdimm/region_devs.c | 9 +++++++++
> include/linux/libnvdimm.h | 2 ++
> 5 files changed, 41 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
> index 7361d00818e2..662036bdc65e 100644
> --- a/drivers/acpi/nfit/core.c
> +++ b/drivers/acpi/nfit/core.c
> @@ -1603,7 +1603,7 @@ static size_t sizeof_nfit_set_info(int num_mappings)
> + num_mappings * sizeof(struct nfit_set_info_map);
> }
>
> -static int cmp_map(const void *m0, const void *m1)
> +static int cmp_map_compat(const void *m0, const void *m1)
> {
> const struct nfit_set_info_map *map0 = m0;
> const struct nfit_set_info_map *map1 = m1;
> @@ -1612,6 +1612,14 @@ static int cmp_map(const void *m0, const void *m1)
> sizeof(u64));
> }
>
> +static int cmp_map(const void *m0, const void *m1)
> +{
> + const struct nfit_set_info_map *map0 = m0;
> + const struct nfit_set_info_map *map1 = m1;
> +
> + return map0->region_offset - map1->region_offset;
> +}
> +
> /* Retrieve the nth entry referencing this spa */
> static struct acpi_nfit_memory_map *memdev_from_spa(
> struct acpi_nfit_desc *acpi_desc, u16 range_index, int n)
> @@ -1667,6 +1675,12 @@ static int acpi_nfit_init_interleave_set(struct acpi_nfit_desc *acpi_desc,
> sort(&info->mapping[0], nr, sizeof(struct nfit_set_info_map),
> cmp_map, NULL);
> nd_set->cookie = nd_fletcher64(info, sizeof_nfit_set_info(nr), 0);
> +
> + /* support namespaces created with the wrong sort order */
> + sort(&info->mapping[0], nr, sizeof(struct nfit_set_info_map),
> + cmp_map_compat, NULL);
> + nd_set->altcookie = nd_fletcher64(info, sizeof_nfit_set_info(nr), 0);
> +
> ndr_desc->nd_set = nd_set;
> devm_kfree(dev, info);
>
> diff --git a/drivers/nvdimm/namespace_devs.c b/drivers/nvdimm/namespace_devs.c
> index ce3e8dfa10ad..1b481a5fb966 100644
> --- a/drivers/nvdimm/namespace_devs.c
> +++ b/drivers/nvdimm/namespace_devs.c
> @@ -1700,6 +1700,7 @@ static int select_pmem_id(struct nd_region *nd_region, u8 *pmem_id)
> struct device *create_namespace_pmem(struct nd_region *nd_region,
> struct nd_namespace_label *nd_label)
> {
> + u64 altcookie = nd_region_interleave_set_altcookie(nd_region);
> u64 cookie = nd_region_interleave_set_cookie(nd_region);
> struct nd_label_ent *label_ent;
> struct nd_namespace_pmem *nspm;
> @@ -1718,7 +1719,11 @@ struct device *create_namespace_pmem(struct nd_region *nd_region,
> if (__le64_to_cpu(nd_label->isetcookie) != cookie) {
> dev_dbg(&nd_region->dev, "invalid cookie in label: %pUb\n",
> nd_label->uuid);
> - return ERR_PTR(-EAGAIN);
> + if (__le64_to_cpu(nd_label->isetcookie) != altcookie)
> + return ERR_PTR(-EAGAIN);
> +
> + dev_dbg(&nd_region->dev, "valid altcookie in label: %pUb\n",
> + nd_label->uuid);
> }
>
> nspm = kzalloc(sizeof(*nspm), GFP_KERNEL);
> @@ -1733,9 +1738,14 @@ struct device *create_namespace_pmem(struct nd_region *nd_region,
> res->name = dev_name(&nd_region->dev);
> res->flags = IORESOURCE_MEM;
>
> - for (i = 0; i < nd_region->ndr_mappings; i++)
> - if (!has_uuid_at_pos(nd_region, nd_label->uuid, cookie, i))
> - break;
> + for (i = 0; i < nd_region->ndr_mappings; i++) {
> + if (has_uuid_at_pos(nd_region, nd_label->uuid, cookie, i))
> + continue;
> + if (has_uuid_at_pos(nd_region, nd_label->uuid, altcookie, i))
> + continue;
> + break;
> + }
> +
> if (i < nd_region->ndr_mappings) {
> struct nvdimm_drvdata *ndd = to_ndd(&nd_region->mapping[i]);
>
> diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
> index 35dd75057e16..2a99c83aa19f 100644
> --- a/drivers/nvdimm/nd.h
> +++ b/drivers/nvdimm/nd.h
> @@ -328,6 +328,7 @@ struct nd_region *to_nd_region(struct device *dev);
> int nd_region_to_nstype(struct nd_region *nd_region);
> int nd_region_register_namespaces(struct nd_region *nd_region, int *err);
> u64 nd_region_interleave_set_cookie(struct nd_region *nd_region);
> +u64 nd_region_interleave_set_altcookie(struct nd_region *nd_region);
> void nvdimm_bus_lock(struct device *dev);
> void nvdimm_bus_unlock(struct device *dev);
> bool is_nvdimm_bus_locked(struct device *dev);
> diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
> index 7cd705f3247c..b7cb5066d961 100644
> --- a/drivers/nvdimm/region_devs.c
> +++ b/drivers/nvdimm/region_devs.c
> @@ -505,6 +505,15 @@ u64 nd_region_interleave_set_cookie(struct nd_region *nd_region)
> return 0;
> }
>
> +u64 nd_region_interleave_set_altcookie(struct nd_region *nd_region)
> +{
> + struct nd_interleave_set *nd_set = nd_region->nd_set;
> +
> + if (nd_set)
> + return nd_set->altcookie;
> + return 0;
> +}
> +
> void nd_mapping_free_labels(struct nd_mapping *nd_mapping)
> {
> struct nd_label_ent *label_ent, *e;
> diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
> index 8458c5351e56..77e7af32543f 100644
> --- a/include/linux/libnvdimm.h
> +++ b/include/linux/libnvdimm.h
> @@ -70,6 +70,8 @@ struct nd_cmd_desc {
>
> struct nd_interleave_set {
> u64 cookie;
> + /* compatibility with initial buggy Linux implementation */
> + u64 altcookie;
> };
>
> struct nd_mapping_desc {
>
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
