2019-09-10 16:13:34

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH v3 0/5] Read-only memremap()

This patch series implements a read-only version of memremap() via
a new MEMREMAP_RO flag. If this is passed in the mapping call, we'll
try to map the memory region as read-only if it doesn't intersect
with an existing mapping. Otherwise, we'll try to fallback to other
flags to try to map the memory that way.

The main use case I have is to map the command-db memory region on
Qualcomm devices with a read-only mapping. It's already a const marked
pointer and the API returns const pointers as well, so this series makes
sure that even stray writes can't modify the memory. To get there we
introduce a devm version of memremap() for a reserved memory region, add
a memremap() flag, and implement support for that flag on arm64.

Changes from v2 (https://lkml.kernel.org/r/[email protected]):
* Added a comment to kerneldoc for the new MEMREMAP_RO flag
* Rebased to v5.3-rc1

Changes from v1:
* Picked up tags and rebased to v5.2-rc3

Stephen Boyd (5):
reserved_mem: Add a devm_memremap_reserved_mem() API
soc: qcom: cmd-db: Migrate to devm_memremap_reserved_mem()
memremap: Add support for read-only memory mappings
arm64: Add support for arch_memremap_ro()
soc: qcom: cmd-db: Map with read-only mappings

Cc: Evan Green <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Cc: Andy Gross <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Catalin Marinas <[email protected]>

arch/arm64/include/asm/io.h | 1 +
drivers/of/of_reserved_mem.c | 45 +++++++++++++++++++++++++++++++++
drivers/soc/qcom/cmd-db.c | 14 +++-------
include/linux/io.h | 1 +
include/linux/of_reserved_mem.h | 6 +++++
kernel/iomem.c | 20 ++++++++++++---
6 files changed, 74 insertions(+), 13 deletions(-)


base-commit: 5f9e832c137075045d15cd6899ab0505cfb2ca4b
--
Sent by a computer through tubes


2019-09-10 19:13:56

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH v3 5/5] soc: qcom: cmd-db: Map with read-only mappings

The command DB is read-only already to the kernel because everything is
const marked once we map it. Let's go one step further and try to map
the memory as read-only in the page tables. This should make it harder
for random code to corrupt the database and change the contents.

Cc: Evan Green <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Cc: Andy Gross <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Dan Williams <[email protected]>
Reviewed-by: Bjorn Andersson <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
drivers/soc/qcom/cmd-db.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/qcom/cmd-db.c b/drivers/soc/qcom/cmd-db.c
index 10a34d26b753..6365e8260282 100644
--- a/drivers/soc/qcom/cmd-db.c
+++ b/drivers/soc/qcom/cmd-db.c
@@ -240,7 +240,8 @@ static int cmd_db_dev_probe(struct platform_device *pdev)
{
int ret = 0;

- cmd_db_header = devm_memremap_reserved_mem(&pdev->dev, MEMREMAP_WB);
+ cmd_db_header = devm_memremap_reserved_mem(&pdev->dev,
+ MEMREMAP_RO | MEMREMAP_WB);
if (IS_ERR(cmd_db_header)) {
ret = PTR_ERR(cmd_db_header);
cmd_db_header = NULL;
--
Sent by a computer through tubes

2019-09-18 22:09:52

by Evan Green

[permalink] [raw]
Subject: Re: [PATCH v3 5/5] soc: qcom: cmd-db: Map with read-only mappings

On Tue, Sep 10, 2019 at 9:09 AM Stephen Boyd <[email protected]> wrote:
>
> The command DB is read-only already to the kernel because everything is
> const marked once we map it. Let's go one step further and try to map
> the memory as read-only in the page tables. This should make it harder
> for random code to corrupt the database and change the contents.
>
> Cc: Evan Green <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Bjorn Andersson <[email protected]>
> Cc: Andy Gross <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: Dan Williams <[email protected]>
> Reviewed-by: Bjorn Andersson <[email protected]>
> Signed-off-by: Stephen Boyd <[email protected]>
> ---
> drivers/soc/qcom/cmd-db.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/qcom/cmd-db.c b/drivers/soc/qcom/cmd-db.c
> index 10a34d26b753..6365e8260282 100644
> --- a/drivers/soc/qcom/cmd-db.c
> +++ b/drivers/soc/qcom/cmd-db.c
> @@ -240,7 +240,8 @@ static int cmd_db_dev_probe(struct platform_device *pdev)
> {
> int ret = 0;
>
> - cmd_db_header = devm_memremap_reserved_mem(&pdev->dev, MEMREMAP_WB);
> + cmd_db_header = devm_memremap_reserved_mem(&pdev->dev,
> + MEMREMAP_RO | MEMREMAP_WB);

It seems weird to have both flags, like: "It's read-only, but if it
ever did get written to somehow, make it writeback".

> if (IS_ERR(cmd_db_header)) {
> ret = PTR_ERR(cmd_db_header);
> cmd_db_header = NULL;
> --
> Sent by a computer through tubes
>