Re: [PATCH 07/22] memremap: move dev_pagemap callbacks into a separate structure

2019-06-13 Thread Logan Gunthorpe



On 2019-06-13 3:43 a.m., Christoph Hellwig wrote:
> The dev_pagemap is a growing too many callbacks.  Move them into a
> separate ops structure so that they are not duplicated for multiple
> instances, and an attacker can't easily overwrite them.
> 
> Signed-off-by: Christoph Hellwig 
> ---
>  drivers/dax/device.c  |  6 +-
>  drivers/nvdimm/pmem.c | 18 +-
>  drivers/pci/p2pdma.c  |  5 -
>  include/linux/memremap.h  | 29 +++--
>  kernel/memremap.c | 12 ++--
>  mm/hmm.c  |  8 ++--
>  tools/testing/nvdimm/test/iomap.c |  2 +-
>  7 files changed, 50 insertions(+), 30 deletions(-)

Looks good to me,

Reviewed-by: Logan Gunthorpe 

Logan


Re: [PATCH 07/22] memremap: move dev_pagemap callbacks into a separate structure

2019-06-13 Thread Jason Gunthorpe
On Thu, Jun 13, 2019 at 11:43:10AM +0200, Christoph Hellwig wrote:
> The dev_pagemap is a growing too many callbacks.  Move them into a
> separate ops structure so that they are not duplicated for multiple
> instances, and an attacker can't easily overwrite them.

Reviewed-by: Jason Gunthorpe 

Jason


[Nouveau] [PATCH 07/22] memremap: move dev_pagemap callbacks into a separate structure

2019-06-13 Thread Christoph Hellwig
The dev_pagemap is a growing too many callbacks.  Move them into a
separate ops structure so that they are not duplicated for multiple
instances, and an attacker can't easily overwrite them.

Signed-off-by: Christoph Hellwig 
---
 drivers/dax/device.c  |  6 +-
 drivers/nvdimm/pmem.c | 18 +-
 drivers/pci/p2pdma.c  |  5 -
 include/linux/memremap.h  | 29 +++--
 kernel/memremap.c | 12 ++--
 mm/hmm.c  |  8 ++--
 tools/testing/nvdimm/test/iomap.c |  2 +-
 7 files changed, 50 insertions(+), 30 deletions(-)

diff --git a/drivers/dax/device.c b/drivers/dax/device.c
index 996d68ff992a..4adab774dade 100644
--- a/drivers/dax/device.c
+++ b/drivers/dax/device.c
@@ -443,6 +443,10 @@ static void dev_dax_kill(void *dev_dax)
kill_dev_dax(dev_dax);
 }
 
+static const struct dev_pagemap_ops dev_dax_pagemap_ops = {
+   .kill   = dev_dax_percpu_kill,
+};
+
 int dev_dax_probe(struct device *dev)
 {
struct dev_dax *dev_dax = to_dev_dax(dev);
@@ -471,7 +475,7 @@ int dev_dax_probe(struct device *dev)
return rc;
 
dev_dax->pgmap.ref = _dax->ref;
-   dev_dax->pgmap.kill = dev_dax_percpu_kill;
+   dev_dax->pgmap.ops = _dax_pagemap_ops;
addr = devm_memremap_pages(dev, _dax->pgmap);
if (IS_ERR(addr)) {
devm_remove_action(dev, dev_dax_percpu_exit, _dax->ref);
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index d9d845077b8b..4efbf184ea68 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -316,7 +316,7 @@ static void pmem_release_queue(void *q)
blk_cleanup_queue(q);
 }
 
-static void pmem_freeze_queue(struct percpu_ref *ref)
+static void pmem_kill(struct percpu_ref *ref)
 {
struct request_queue *q;
 
@@ -339,19 +339,27 @@ static void pmem_release_pgmap_ops(void *__pgmap)
dev_pagemap_put_ops();
 }
 
-static void fsdax_pagefree(struct page *page, void *data)
+static void pmem_fsdax_page_free(struct page *page, void *data)
 {
wake_up_var(>_refcount);
 }
 
+static const struct dev_pagemap_ops fsdax_pagemap_ops = {
+   .page_free  = pmem_fsdax_page_free,
+   .kill   = pmem_kill,
+};
+
+static const struct dev_pagemap_ops pmem_legacy_pagemap_ops = {
+   .kill   = pmem_kill,
+};
+
 static int setup_pagemap_fsdax(struct device *dev, struct dev_pagemap *pgmap)
 {
dev_pagemap_get_ops();
if (devm_add_action_or_reset(dev, pmem_release_pgmap_ops, pgmap))
return -ENOMEM;
pgmap->type = MEMORY_DEVICE_FS_DAX;
-   pgmap->page_free = fsdax_pagefree;
-
+   pgmap->ops = _pagemap_ops;
return 0;
 }
 
@@ -412,7 +420,6 @@ static int pmem_attach_disk(struct device *dev,
 
pmem->pfn_flags = PFN_DEV;
pmem->pgmap.ref = >q_usage_counter;
-   pmem->pgmap.kill = pmem_freeze_queue;
if (is_nd_pfn(dev)) {
if (setup_pagemap_fsdax(dev, >pgmap))
return -ENOMEM;
@@ -433,6 +440,7 @@ static int pmem_attach_disk(struct device *dev,
pmem->pfn_flags |= PFN_MAP;
memcpy(_res, >pgmap.res, sizeof(bb_res));
} else {
+   pmem->pgmap.ops = _legacy_pagemap_ops;
addr = devm_memremap(dev, pmem->phys_addr,
pmem->size, ARCH_MEMREMAP_PMEM);
memcpy(_res, >res, sizeof(bb_res));
diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
index 742928d0053e..6e76380f5b97 100644
--- a/drivers/pci/p2pdma.c
+++ b/drivers/pci/p2pdma.c
@@ -150,6 +150,10 @@ static int pci_p2pdma_setup(struct pci_dev *pdev)
return error;
 }
 
+static const struct dev_pagemap_ops pci_p2pdma_pagemap_ops = {
+   .kill   = pci_p2pdma_percpu_kill,
+};
+
 /**
  * pci_p2pdma_add_resource - add memory for use as p2p memory
  * @pdev: the device to add the memory to
@@ -196,7 +200,6 @@ int pci_p2pdma_add_resource(struct pci_dev *pdev, int bar, 
size_t size,
pgmap->type = MEMORY_DEVICE_PCI_P2PDMA;
pgmap->pci_p2pdma_bus_offset = pci_bus_address(pdev, bar) -
pci_resource_start(pdev, bar);
-   pgmap->kill = pci_p2pdma_percpu_kill;
 
addr = devm_memremap_pages(>dev, pgmap);
if (IS_ERR(addr)) {
diff --git a/include/linux/memremap.h b/include/linux/memremap.h
index f0628660d541..5f7f40875b35 100644
--- a/include/linux/memremap.h
+++ b/include/linux/memremap.h
@@ -63,39 +63,40 @@ enum memory_type {
MEMORY_DEVICE_PCI_P2PDMA,
 };
 
-/*
- * Additional notes about MEMORY_DEVICE_PRIVATE may be found in
- * include/linux/hmm.h and Documentation/vm/hmm.rst. There is also a brief
- * explanation in include/linux/memory_hotplug.h.
- *
- * The page_free() callback is called once the page refcount reaches 1
- * (ZONE_DEVICE pages never reach 0 refcount unless there is a refcount bug.
- * This