[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAPcyv4g_W5KJq8vrVdi8-Hij4zQLLjZdZMkmP7oWABSrmGfLRw@mail.gmail.com>
Date: Wed, 17 Oct 2018 17:49:25 -0700
From: Dan Williams <dan.j.williams@...el.com>
To: Kees Cook <keescook@...omium.org>
Cc: Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Anton Vorontsov <anton@...msg.org>,
Colin Cross <ccross@...roid.com>,
"Luck, Tony" <tony.luck@...el.com>, joel@...lfernandes.org
Subject: Re: [PATCH] pstore/ram: Clarify resource reservation labels
On Wed, Oct 17, 2018 at 5:29 PM Kees Cook <keescook@...omium.org> wrote:
>
> When ramoops reserved a memory region in the kernel, it had an unhelpful
> label of "persistent_memory". When reading /proc/iomem, it would be
> repeated many times, did not hint that it was ramoops in particular,
> and didn't clarify very much about what each was used for:
>
> 400000000-407ffffff : Persistent Memory (legacy)
> 400000000-400000fff : persistent_memory
> 400001000-400001fff : persistent_memory
> ...
> 4000ff000-4000fffff : persistent_memory
>
> Instead, this adds meaningful labels for how the various regions are
> being used:
>
> 400000000-407ffffff : Persistent Memory (legacy)
> 400000000-400000fff : ramoops:dump(0/252)
> 400001000-400001fff : ramoops:dump(1/252)
> ...
> 4000fc000-4000fcfff : ramoops:dump(252/252)
> 4000fd000-4000fdfff : ramoops:console
> 4000fe000-4000fe3ff : ramoops:ftrace(0/3)
> 4000fe400-4000fe7ff : ramoops:ftrace(1/3)
> 4000fe800-4000febff : ramoops:ftrace(2/3)
> 4000fec00-4000fefff : ramoops:ftrace(3/3)
> 4000ff000-4000fffff : ramoops:pmsg
Hopefully ramoops is doing request_region() before trying to do
anything with its ranges, because it's going to collide with the pmem
driver doing a request_region(). If we want to have pstore use pmem as
a backing store that's a new drivers/nvdimm/ namespace personality
driver to turn around and register a persistent memory range with
pstore rather than the pmem block-device driver.
>
> Signed-off-by: Kees Cook <keescook@...omium.org>
> ---
> fs/pstore/ram.c | 16 +++++++++++++---
> fs/pstore/ram_core.c | 11 +++++++----
> include/linux/pstore_ram.h | 3 ++-
> 3 files changed, 22 insertions(+), 8 deletions(-)
>
> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
> index 6ea9cd801044..ffcff6516e89 100644
> --- a/fs/pstore/ram.c
> +++ b/fs/pstore/ram.c
> @@ -587,9 +587,16 @@ static int ramoops_init_przs(const char *name,
> goto fail;
>
> for (i = 0; i < *cnt; i++) {
> + char *label;
> +
> + if (*cnt == 1)
> + label = kasprintf(GFP_KERNEL, "ramoops:%s", name);
> + else
> + label = kasprintf(GFP_KERNEL, "ramoops:%s(%d/%d)",
> + name, i, *cnt - 1);
> prz_ar[i] = persistent_ram_new(*paddr, zone_sz, sig,
> - &cxt->ecc_info,
> - cxt->memtype, flags);
> + &cxt->ecc_info,
> + cxt->memtype, flags, label);
> if (IS_ERR(prz_ar[i])) {
> err = PTR_ERR(prz_ar[i]);
> dev_err(dev, "failed to request %s mem region (0x%zx@...llx): %d\n",
> @@ -619,6 +626,8 @@ static int ramoops_init_prz(const char *name,
> struct persistent_ram_zone **prz,
> phys_addr_t *paddr, size_t sz, u32 sig)
> {
> + char *label;
> +
> if (!sz)
> return 0;
>
> @@ -629,8 +638,9 @@ static int ramoops_init_prz(const char *name,
> return -ENOMEM;
> }
>
> + label = kasprintf(GFP_KERNEL, "ramoops:%s", name);
> *prz = persistent_ram_new(*paddr, sz, sig, &cxt->ecc_info,
> - cxt->memtype, 0);
> + cxt->memtype, 0, label);
> if (IS_ERR(*prz)) {
> int err = PTR_ERR(*prz);
>
> diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
> index 0792595ebcfb..12e21f789194 100644
> --- a/fs/pstore/ram_core.c
> +++ b/fs/pstore/ram_core.c
> @@ -438,11 +438,11 @@ static void *persistent_ram_vmap(phys_addr_t start, size_t size,
> }
>
> static void *persistent_ram_iomap(phys_addr_t start, size_t size,
> - unsigned int memtype)
> + unsigned int memtype, char *label)
> {
> void *va;
>
> - if (!request_mem_region(start, size, "persistent_ram")) {
> + if (!request_mem_region(start, size, label ?: "ramoops")) {
> pr_err("request mem region (0x%llx@...llx) failed\n",
> (unsigned long long)size, (unsigned long long)start);
> return NULL;
> @@ -470,7 +470,8 @@ static int persistent_ram_buffer_map(phys_addr_t start, phys_addr_t size,
> if (pfn_valid(start >> PAGE_SHIFT))
> prz->vaddr = persistent_ram_vmap(start, size, memtype);
> else
> - prz->vaddr = persistent_ram_iomap(start, size, memtype);
> + prz->vaddr = persistent_ram_iomap(start, size, memtype,
> + prz->label);
>
> if (!prz->vaddr) {
> pr_err("%s: Failed to map 0x%llx pages at 0x%llx\n", __func__,
> @@ -541,12 +542,13 @@ void persistent_ram_free(struct persistent_ram_zone *prz)
> prz->ecc_info.par = NULL;
>
> persistent_ram_free_old(prz);
> + kfree(prz->label);
> kfree(prz);
> }
>
> struct persistent_ram_zone *persistent_ram_new(phys_addr_t start, size_t size,
> u32 sig, struct persistent_ram_ecc_info *ecc_info,
> - unsigned int memtype, u32 flags)
> + unsigned int memtype, u32 flags, char *label)
> {
> struct persistent_ram_zone *prz;
> int ret = -ENOMEM;
> @@ -560,6 +562,7 @@ struct persistent_ram_zone *persistent_ram_new(phys_addr_t start, size_t size,
> /* Initialize general buffer state. */
> raw_spin_lock_init(&prz->buffer_lock);
> prz->flags = flags;
> + prz->label = label;
>
> ret = persistent_ram_buffer_map(start, size, prz, memtype);
> if (ret)
> diff --git a/include/linux/pstore_ram.h b/include/linux/pstore_ram.h
> index e6d226464838..602d64725222 100644
> --- a/include/linux/pstore_ram.h
> +++ b/include/linux/pstore_ram.h
> @@ -46,6 +46,7 @@ struct persistent_ram_zone {
> phys_addr_t paddr;
> size_t size;
> void *vaddr;
> + char *label;
> struct persistent_ram_buffer *buffer;
> size_t buffer_size;
> u32 flags;
> @@ -65,7 +66,7 @@ struct persistent_ram_zone {
>
> struct persistent_ram_zone *persistent_ram_new(phys_addr_t start, size_t size,
> u32 sig, struct persistent_ram_ecc_info *ecc_info,
> - unsigned int memtype, u32 flags);
> + unsigned int memtype, u32 flags, char *label);
> void persistent_ram_free(struct persistent_ram_zone *prz);
> void persistent_ram_zap(struct persistent_ram_zone *prz);
>
> --
> 2.17.1
>
>
> --
> Kees Cook
> Pixel Security
Powered by blists - more mailing lists