[v4,2/6] libnvdimm/namespace: Validate namespace start addr and size
diff mbox series

Message ID 20200120140749.69549-3-aneesh.kumar@linux.ibm.com
State New
Headers show
Series
  • Validating namespace size and start address attributes.
Related show

Commit Message

Aneesh Kumar K.V Jan. 20, 2020, 2:07 p.m. UTC
Make sure namespace start addr and size are properly aligned as per architecture
restrictions. If the namespace is not aligned kernel mark the namespace 'disabled'

Architectures like ppc64 use different page size than PAGE_SIZE to map
direct-map address range. The kernel needs to make sure the namespace size is aligned
correctly for the direct-map page size.

kernel log will contain information as below.

[    5.810939] nd_pmem namespace0.1: invalid size/SPA
[    5.810969] nd_pmem: probe of namespace0.1 failed with error -95

and the namespace will be marked 'disabled'

  {
    "dev":"namespace0.1",
    "mode":"fsdax",
    "map":"mem",
    "size":1071644672,
    "uuid":"25577a00-c012-421d-89ca-3ee189e08848",
    "sector_size":512,
    "state":"disabled"
  },

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
 drivers/nvdimm/namespace_devs.c | 50 +++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

Comments

Dan Williams Jan. 25, 2020, 1:55 a.m. UTC | #1
On Mon, Jan 20, 2020 at 6:08 AM Aneesh Kumar K.V
<aneesh.kumar@linux.ibm.com> wrote:
>
> Make sure namespace start addr and size are properly aligned as per architecture
> restrictions. If the namespace is not aligned kernel mark the namespace 'disabled'
>
> Architectures like ppc64 use different page size than PAGE_SIZE to map
> direct-map address range. The kernel needs to make sure the namespace size is aligned
> correctly for the direct-map page size.
>
> kernel log will contain information as below.
>
> [    5.810939] nd_pmem namespace0.1: invalid size/SPA
> [    5.810969] nd_pmem: probe of namespace0.1 failed with error -95
>
> and the namespace will be marked 'disabled'
>
>   {
>     "dev":"namespace0.1",
>     "mode":"fsdax",
>     "map":"mem",
>     "size":1071644672,
>     "uuid":"25577a00-c012-421d-89ca-3ee189e08848",
>     "sector_size":512,
>     "state":"disabled"
>   },
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
>  drivers/nvdimm/namespace_devs.c | 50 +++++++++++++++++++++++++++++++++
>  1 file changed, 50 insertions(+)
>
> diff --git a/drivers/nvdimm/namespace_devs.c b/drivers/nvdimm/namespace_devs.c
> index 032dc61725ff..0e2c90730ce3 100644
> --- a/drivers/nvdimm/namespace_devs.c
> +++ b/drivers/nvdimm/namespace_devs.c
> @@ -1113,6 +1113,51 @@ resource_size_t nvdimm_namespace_capacity(struct nd_namespace_common *ndns)
>  }
>  EXPORT_SYMBOL(nvdimm_namespace_capacity);
>
> +static bool nvdimm_valid_namespace(struct device *dev,
> +               struct nd_namespace_common *ndns, resource_size_t size)
> +{
> +       struct device *ndns_dev = &ndns->dev;
> +       struct nd_region *nd_region = to_nd_region(ndns->dev.parent);
> +       unsigned long map_size = arch_namespace_map_size();
> +       struct resource *res;
> +       u32 remainder;
> +
> +       /*
> +        * Don't validate the start and size for blk namespace type
> +        */

If you're going to comment I'd prefer a comment that explains why, not
what. E.g. "blk namespaces are ioremap()'d with small page aligned
apertures"

> +       if (is_namespace_blk(ndns_dev))
> +               return true;
> +
> +       /*
> +        * For btt and raw namespace kernel use ioremap. Assume both can work
> +        * with PAGE_SIZE alignment.
> +        */
> +       if (is_nd_btt(dev) || is_namespace_io(ndns_dev))
> +               map_size = PAGE_SIZE;
> +
> +       div_u64_rem(size, map_size * nd_region->ndr_mappings, &remainder);

Not all regions have mappings see e820_register_one(), so this is a
divide by zero.

I think this is mixing 2 different constraints. There's the soft
constraint of wanting aliased pmem capacity to remain aligned relative
to blk capacity, and then there's the hard constraint where the kernel
just can't use devm_memremap_pages() for the given capacity alignment.

I think this routine should be called:

    nvdimm_validate_geometry()

...and only check for:

    if (pmem_should_map_pages() || is_nd_pfn() || is_nd_dax())

...i.e namespaces are mapped by memremap_pages() and compare that
against memremap_compat_align().

Patch
diff mbox series

diff --git a/drivers/nvdimm/namespace_devs.c b/drivers/nvdimm/namespace_devs.c
index 032dc61725ff..0e2c90730ce3 100644
--- a/drivers/nvdimm/namespace_devs.c
+++ b/drivers/nvdimm/namespace_devs.c
@@ -1113,6 +1113,51 @@  resource_size_t nvdimm_namespace_capacity(struct nd_namespace_common *ndns)
 }
 EXPORT_SYMBOL(nvdimm_namespace_capacity);
 
+static bool nvdimm_valid_namespace(struct device *dev,
+		struct nd_namespace_common *ndns, resource_size_t size)
+{
+	struct device *ndns_dev = &ndns->dev;
+	struct nd_region *nd_region = to_nd_region(ndns->dev.parent);
+	unsigned long map_size = arch_namespace_map_size();
+	struct resource *res;
+	u32 remainder;
+
+	/*
+	 * Don't validate the start and size for blk namespace type
+	 */
+	if (is_namespace_blk(ndns_dev))
+		return true;
+
+	/*
+	 * For btt and raw namespace kernel use ioremap. Assume both can work
+	 * with PAGE_SIZE alignment.
+	 */
+	if (is_nd_btt(dev) || is_namespace_io(ndns_dev))
+		map_size = PAGE_SIZE;
+
+	div_u64_rem(size, map_size * nd_region->ndr_mappings, &remainder);
+	if (remainder)
+		return false;
+
+	if (is_namespace_pmem(ndns_dev)) {
+		struct nd_namespace_pmem *nspm = to_nd_namespace_pmem(ndns_dev);
+
+		res = &nspm->nsio.res;
+	} else if (is_namespace_io(ndns_dev)) {
+		struct nd_namespace_io *nsio = to_nd_namespace_io(ndns_dev);
+
+		res = &nsio->res;
+	} else
+		/* cannot reach */
+		return false;
+
+	div_u64_rem(res->start, map_size * nd_region->ndr_mappings, &remainder);
+	if (remainder)
+		return false;
+
+	return true;
+}
+
 bool nvdimm_namespace_locked(struct nd_namespace_common *ndns)
 {
 	int i;
@@ -1739,6 +1784,11 @@  struct nd_namespace_common *nvdimm_namespace_common_probe(struct device *dev)
 		return ERR_PTR(-ENODEV);
 	}
 
+	if (!nvdimm_valid_namespace(dev, ndns, size)) {
+		dev_err(&ndns->dev, "invalid size/SPA");
+		return ERR_PTR(-EOPNOTSUPP);
+	}
+
 	if (is_namespace_pmem(&ndns->dev)) {
 		struct nd_namespace_pmem *nspm;