* [PATCH v2 1/2] nvdimm/pfn_dev: Prevent the creation of zero-sized namespaces
@ 2023-08-09 5:35 Aneesh Kumar K.V
2023-08-09 5:35 ` [PATCH v2 2/2] nvdimm/pfn_dev: Avoid unnecessary endian conversion Aneesh Kumar K.V
2023-08-09 13:56 ` [PATCH v2 1/2] nvdimm/pfn_dev: Prevent the creation of zero-sized namespaces Jeff Moyer
0 siblings, 2 replies; 3+ messages in thread
From: Aneesh Kumar K.V @ 2023-08-09 5:35 UTC (permalink / raw)
To: nvdimm, dan.j.williams, vishal.l.verma; +Cc: Jeff Moyer, Aneesh Kumar K.V
On architectures that have different page size values used for kernel
direct mapping and userspace mappings, the user can end up creating zero-sized
namespaces as shown below
:/sys/bus/nd/devices/region1# cat align
0x1000000
/sys/bus/nd/devices/region1# echo 0x200000 > align
/sys/bus/nd/devices/region1/dax1.0# cat supported_alignments
65536 16777216
$ ndctl create-namespace -r region1 -m devdax -s 18M --align 64K
{
"dev":"namespace1.0",
"mode":"devdax",
"map":"dev",
"size":0,
"uuid":"3094329a-0c66-4905-847e-357223e56ab0",
"daxregion":{
"id":1,
"size":0,
"align":65536
},
"align":65536
}
similarily for fsdax
$ ndctl create-namespace -r region1 -m fsdax -s 18M --align 64K
{
"dev":"namespace1.0",
"mode":"fsdax",
"map":"dev",
"size":0,
"uuid":"45538a6f-dec7-405d-b1da-2a4075e06232",
"sector_size":512,
"align":65536,
"blockdev":"pmem1"
}
In commit 9ffc1d19fc4a ("mm/memremap_pages: Introduce memremap_compat_align()")
memremap_compat_align was added to make sure the kernel always creates
namespaces with 16MB alignment. But the user can still override the
region alignment and no input validation is done in the region alignment
values to retain the flexibility user had before. However, the kernel
ensures that only part of the namespace that can be mapped via kernel
direct mapping page size is enabled. This is achieved by tracking the
unmapped part of the namespace in pfn_sb->end_trunc. The kernel also
ensures that the start address of the namespace is also aligned to the
kernel direct mapping page size.
Depending on the user request, the kernel implements userspace mapping
alignment by updating pfn device alignment attribute and this value is
used to adjust the start address for userspace mappings. This is tracked
in pfn_sb->dataoff. Hence the available size for userspace mapping is:
usermapping_size = size of the namespace - pfn_sb->end_trun - pfn_sb->dataoff
If the kernel finds the user mapping size zero then don't allow the
creation of namespace.
After fix:
$ ndctl create-namespace -f -r region1 -m devdax -s 18M --align 64K
libndctl: ndctl_dax_enable: dax1.1: failed to enable
Error: namespace1.2: failed to enable
failed to create namespace: No such device or address
And existing zero sized namespace will be marked disabled.
root@ltczz75-lp2:/home/kvaneesh# ndctl list -N -r region1 -i
[
{
"dev":"namespace1.0",
"mode":"raw",
"size":18874368,
"uuid":"94a90fb0-8e78-4fb6-a759-ffc62f9fa181",
"sector_size":512,
"state":"disabled"
},
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
Changes from v1:
* Use resource_size() helper instead of opencoding it
drivers/nvdimm/pfn_devs.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c
index af7d9301520c..0777b1626f6c 100644
--- a/drivers/nvdimm/pfn_devs.c
+++ b/drivers/nvdimm/pfn_devs.c
@@ -452,8 +452,9 @@ int nd_pfn_validate(struct nd_pfn *nd_pfn, const char *sig)
u64 checksum, offset;
struct resource *res;
enum nd_pfn_mode mode;
+ resource_size_t res_size;
struct nd_namespace_io *nsio;
- unsigned long align, start_pad;
+ unsigned long align, start_pad, end_trunc;
struct nd_pfn_sb *pfn_sb = nd_pfn->pfn_sb;
struct nd_namespace_common *ndns = nd_pfn->ndns;
const uuid_t *parent_uuid = nd_dev_to_uuid(&ndns->dev);
@@ -503,6 +504,7 @@ int nd_pfn_validate(struct nd_pfn *nd_pfn, const char *sig)
align = le32_to_cpu(pfn_sb->align);
offset = le64_to_cpu(pfn_sb->dataoff);
start_pad = le32_to_cpu(pfn_sb->start_pad);
+ end_trunc = le32_to_cpu(pfn_sb->end_trunc);
if (align == 0)
align = 1UL << ilog2(offset);
mode = le32_to_cpu(pfn_sb->mode);
@@ -584,7 +586,8 @@ int nd_pfn_validate(struct nd_pfn *nd_pfn, const char *sig)
*/
nsio = to_nd_namespace_io(&ndns->dev);
res = &nsio->res;
- if (offset >= resource_size(res)) {
+ res_size = resource_size(res);
+ if (offset >= res_size) {
dev_err(&nd_pfn->dev, "pfn array size exceeds capacity of %s\n",
dev_name(&ndns->dev));
return -EOPNOTSUPP;
@@ -610,6 +613,10 @@ int nd_pfn_validate(struct nd_pfn *nd_pfn, const char *sig)
return -EOPNOTSUPP;
}
+ if (offset >= (res_size - start_pad - end_trunc)) {
+ dev_err(&nd_pfn->dev, "bad offset with small namespace\n");
+ return -EOPNOTSUPP;
+ }
return 0;
}
EXPORT_SYMBOL(nd_pfn_validate);
@@ -810,7 +817,8 @@ static int nd_pfn_init(struct nd_pfn *nd_pfn)
else
return -ENXIO;
- if (offset >= size) {
+ if (offset >= (size - end_trunc)) {
+ /* This results in zero size devices */
dev_err(&nd_pfn->dev, "%s unable to satisfy requested alignment\n",
dev_name(&ndns->dev));
return -ENXIO;
--
2.41.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH v2 2/2] nvdimm/pfn_dev: Avoid unnecessary endian conversion
2023-08-09 5:35 [PATCH v2 1/2] nvdimm/pfn_dev: Prevent the creation of zero-sized namespaces Aneesh Kumar K.V
@ 2023-08-09 5:35 ` Aneesh Kumar K.V
2023-08-09 13:56 ` [PATCH v2 1/2] nvdimm/pfn_dev: Prevent the creation of zero-sized namespaces Jeff Moyer
1 sibling, 0 replies; 3+ messages in thread
From: Aneesh Kumar K.V @ 2023-08-09 5:35 UTC (permalink / raw)
To: nvdimm, dan.j.williams, vishal.l.verma; +Cc: Jeff Moyer, Aneesh Kumar K.V
use the local variable that already have the converted values.
No functional change in this patch.
Reviewed-by: Jeff Moyer <jmoyer@redhat.com>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
drivers/nvdimm/pfn_devs.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c
index 0777b1626f6c..c662d8964802 100644
--- a/drivers/nvdimm/pfn_devs.c
+++ b/drivers/nvdimm/pfn_devs.c
@@ -601,14 +601,12 @@ int nd_pfn_validate(struct nd_pfn *nd_pfn, const char *sig)
return -EOPNOTSUPP;
}
- if (!IS_ALIGNED(res->start + le32_to_cpu(pfn_sb->start_pad),
- memremap_compat_align())) {
+ if (!IS_ALIGNED(res->start + start_pad, memremap_compat_align())) {
dev_err(&nd_pfn->dev, "resource start misaligned\n");
return -EOPNOTSUPP;
}
- if (!IS_ALIGNED(res->end + 1 - le32_to_cpu(pfn_sb->end_trunc),
- memremap_compat_align())) {
+ if (!IS_ALIGNED(res->end + 1 - end_trunc, memremap_compat_align())) {
dev_err(&nd_pfn->dev, "resource end misaligned\n");
return -EOPNOTSUPP;
}
--
2.41.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2 1/2] nvdimm/pfn_dev: Prevent the creation of zero-sized namespaces
2023-08-09 5:35 [PATCH v2 1/2] nvdimm/pfn_dev: Prevent the creation of zero-sized namespaces Aneesh Kumar K.V
2023-08-09 5:35 ` [PATCH v2 2/2] nvdimm/pfn_dev: Avoid unnecessary endian conversion Aneesh Kumar K.V
@ 2023-08-09 13:56 ` Jeff Moyer
1 sibling, 0 replies; 3+ messages in thread
From: Jeff Moyer @ 2023-08-09 13:56 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: nvdimm, dan.j.williams, vishal.l.verma
"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
> On architectures that have different page size values used for kernel
> direct mapping and userspace mappings, the user can end up creating zero-sized
> namespaces as shown below
>
> :/sys/bus/nd/devices/region1# cat align
> 0x1000000
> /sys/bus/nd/devices/region1# echo 0x200000 > align
> /sys/bus/nd/devices/region1/dax1.0# cat supported_alignments
> 65536 16777216
> $ ndctl create-namespace -r region1 -m devdax -s 18M --align 64K
> {
> "dev":"namespace1.0",
> "mode":"devdax",
> "map":"dev",
> "size":0,
> "uuid":"3094329a-0c66-4905-847e-357223e56ab0",
> "daxregion":{
> "id":1,
> "size":0,
> "align":65536
> },
> "align":65536
> }
> similarily for fsdax
>
> $ ndctl create-namespace -r region1 -m fsdax -s 18M --align 64K
> {
> "dev":"namespace1.0",
> "mode":"fsdax",
> "map":"dev",
> "size":0,
> "uuid":"45538a6f-dec7-405d-b1da-2a4075e06232",
> "sector_size":512,
> "align":65536,
> "blockdev":"pmem1"
> }
>
> In commit 9ffc1d19fc4a ("mm/memremap_pages: Introduce memremap_compat_align()")
> memremap_compat_align was added to make sure the kernel always creates
> namespaces with 16MB alignment. But the user can still override the
> region alignment and no input validation is done in the region alignment
> values to retain the flexibility user had before. However, the kernel
> ensures that only part of the namespace that can be mapped via kernel
> direct mapping page size is enabled. This is achieved by tracking the
> unmapped part of the namespace in pfn_sb->end_trunc. The kernel also
> ensures that the start address of the namespace is also aligned to the
> kernel direct mapping page size.
>
> Depending on the user request, the kernel implements userspace mapping
> alignment by updating pfn device alignment attribute and this value is
> used to adjust the start address for userspace mappings. This is tracked
> in pfn_sb->dataoff. Hence the available size for userspace mapping is:
>
> usermapping_size = size of the namespace - pfn_sb->end_trun - pfn_sb->dataoff
>
> If the kernel finds the user mapping size zero then don't allow the
> creation of namespace.
>
> After fix:
> $ ndctl create-namespace -f -r region1 -m devdax -s 18M --align 64K
> libndctl: ndctl_dax_enable: dax1.1: failed to enable
> Error: namespace1.2: failed to enable
>
> failed to create namespace: No such device or address
>
> And existing zero sized namespace will be marked disabled.
> root@ltczz75-lp2:/home/kvaneesh# ndctl list -N -r region1 -i
> [
> {
> "dev":"namespace1.0",
> "mode":"raw",
> "size":18874368,
> "uuid":"94a90fb0-8e78-4fb6-a759-ffc62f9fa181",
> "sector_size":512,
> "state":"disabled"
> },
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
> Changes from v1:
> * Use resource_size() helper instead of opencoding it
>
> drivers/nvdimm/pfn_devs.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c
> index af7d9301520c..0777b1626f6c 100644
> --- a/drivers/nvdimm/pfn_devs.c
> +++ b/drivers/nvdimm/pfn_devs.c
> @@ -452,8 +452,9 @@ int nd_pfn_validate(struct nd_pfn *nd_pfn, const char *sig)
> u64 checksum, offset;
> struct resource *res;
> enum nd_pfn_mode mode;
> + resource_size_t res_size;
> struct nd_namespace_io *nsio;
> - unsigned long align, start_pad;
> + unsigned long align, start_pad, end_trunc;
> struct nd_pfn_sb *pfn_sb = nd_pfn->pfn_sb;
> struct nd_namespace_common *ndns = nd_pfn->ndns;
> const uuid_t *parent_uuid = nd_dev_to_uuid(&ndns->dev);
> @@ -503,6 +504,7 @@ int nd_pfn_validate(struct nd_pfn *nd_pfn, const char *sig)
> align = le32_to_cpu(pfn_sb->align);
> offset = le64_to_cpu(pfn_sb->dataoff);
> start_pad = le32_to_cpu(pfn_sb->start_pad);
> + end_trunc = le32_to_cpu(pfn_sb->end_trunc);
> if (align == 0)
> align = 1UL << ilog2(offset);
> mode = le32_to_cpu(pfn_sb->mode);
> @@ -584,7 +586,8 @@ int nd_pfn_validate(struct nd_pfn *nd_pfn, const char *sig)
> */
> nsio = to_nd_namespace_io(&ndns->dev);
> res = &nsio->res;
> - if (offset >= resource_size(res)) {
> + res_size = resource_size(res);
> + if (offset >= res_size) {
> dev_err(&nd_pfn->dev, "pfn array size exceeds capacity of %s\n",
> dev_name(&ndns->dev));
> return -EOPNOTSUPP;
> @@ -610,6 +613,10 @@ int nd_pfn_validate(struct nd_pfn *nd_pfn, const char *sig)
> return -EOPNOTSUPP;
> }
>
> + if (offset >= (res_size - start_pad - end_trunc)) {
> + dev_err(&nd_pfn->dev, "bad offset with small namespace\n");
> + return -EOPNOTSUPP;
> + }
> return 0;
> }
> EXPORT_SYMBOL(nd_pfn_validate);
> @@ -810,7 +817,8 @@ static int nd_pfn_init(struct nd_pfn *nd_pfn)
> else
> return -ENXIO;
>
> - if (offset >= size) {
> + if (offset >= (size - end_trunc)) {
> + /* This results in zero size devices */
> dev_err(&nd_pfn->dev, "%s unable to satisfy requested alignment\n",
> dev_name(&ndns->dev));
> return -ENXIO;
Reviewed-by: Jeff Moyer <jmoyer@redhat.com>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-08-09 13:50 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-09 5:35 [PATCH v2 1/2] nvdimm/pfn_dev: Prevent the creation of zero-sized namespaces Aneesh Kumar K.V
2023-08-09 5:35 ` [PATCH v2 2/2] nvdimm/pfn_dev: Avoid unnecessary endian conversion Aneesh Kumar K.V
2023-08-09 13:56 ` [PATCH v2 1/2] nvdimm/pfn_dev: Prevent the creation of zero-sized namespaces Jeff Moyer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox