From 959217c84c2765434cd2cbf3d7ffdc09e90834f9 Mon Sep 17 00:00:00 2001 From: Li Pengcheng Date: Sat, 5 Nov 2016 10:15:59 +0800 Subject: pstore: Actually give up during locking failure Without a return after the pr_err(), dumps will collide when two threads call pstore_dump() at the same time. Signed-off-by: Liu Hailong Signed-off-by: Li Pengcheng Signed-off-by: Li Zhong [kees: improved commit message] Signed-off-by: Kees Cook --- fs/pstore/platform.c | 1 + 1 file changed, 1 insertion(+) (limited to 'fs/pstore') diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c index 14984d902a99..60e6db6f5da2 100644 --- a/fs/pstore/platform.c +++ b/fs/pstore/platform.c @@ -493,6 +493,7 @@ static void pstore_dump(struct kmsg_dumper *dumper, if (!is_locked) { pr_err("pstore dump routine blocked in %s path, may corrupt error record\n" , in_nmi() ? "NMI" : why); + return; } } else { spin_lock_irqsave(&psinfo->buf_lock, flags); -- cgit v1.2.3 From 109704492ef637956265ec2eb72ae7b3b39eb6f4 Mon Sep 17 00:00:00 2001 From: Joel Fernandes Date: Thu, 20 Oct 2016 00:34:00 -0700 Subject: pstore: Make spinlock per zone instead of global Currently pstore has a global spinlock for all zones. Since the zones are independent and modify different areas of memory, there's no need to have a global lock, so we should use a per-zone lock as introduced here. Also, when ramoops's ftrace use-case has a FTRACE_PER_CPU flag introduced later, which splits the ftrace memory area into a single zone per CPU, it will eliminate the need for locking. In preparation for this, make the locking optional. Signed-off-by: Joel Fernandes [kees: updated commit message] Signed-off-by: Kees Cook --- fs/pstore/ram_core.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'fs/pstore') diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c index 3975deec02f8..cb92055e6016 100644 --- a/fs/pstore/ram_core.c +++ b/fs/pstore/ram_core.c @@ -48,8 +48,6 @@ static inline size_t buffer_start(struct persistent_ram_zone *prz) return atomic_read(&prz->buffer->start); } -static DEFINE_RAW_SPINLOCK(buffer_lock); - /* increase and wrap the start pointer, returning the old value */ static size_t buffer_start_add(struct persistent_ram_zone *prz, size_t a) { @@ -57,7 +55,7 @@ static size_t buffer_start_add(struct persistent_ram_zone *prz, size_t a) int new; unsigned long flags; - raw_spin_lock_irqsave(&buffer_lock, flags); + raw_spin_lock_irqsave(&prz->buffer_lock, flags); old = atomic_read(&prz->buffer->start); new = old + a; @@ -65,7 +63,7 @@ static size_t buffer_start_add(struct persistent_ram_zone *prz, size_t a) new -= prz->buffer_size; atomic_set(&prz->buffer->start, new); - raw_spin_unlock_irqrestore(&buffer_lock, flags); + raw_spin_unlock_irqrestore(&prz->buffer_lock, flags); return old; } @@ -77,7 +75,7 @@ static void buffer_size_add(struct persistent_ram_zone *prz, size_t a) size_t new; unsigned long flags; - raw_spin_lock_irqsave(&buffer_lock, flags); + raw_spin_lock_irqsave(&prz->buffer_lock, flags); old = atomic_read(&prz->buffer->size); if (old == prz->buffer_size) @@ -89,7 +87,7 @@ static void buffer_size_add(struct persistent_ram_zone *prz, size_t a) atomic_set(&prz->buffer->size, new); exit: - raw_spin_unlock_irqrestore(&buffer_lock, flags); + raw_spin_unlock_irqrestore(&prz->buffer_lock, flags); } static void notrace persistent_ram_encode_rs8(struct persistent_ram_zone *prz, @@ -493,6 +491,7 @@ static int persistent_ram_post_init(struct persistent_ram_zone *prz, u32 sig, prz->buffer->sig = sig; persistent_ram_zap(prz); + prz->buffer_lock = __RAW_SPIN_LOCK_UNLOCKED(buffer_lock); return 0; } -- cgit v1.2.3 From d8991f51e55fd3c5b8ad055653a5998e53d4ec91 Mon Sep 17 00:00:00 2001 From: Joel Fernandes Date: Thu, 20 Oct 2016 00:34:02 -0700 Subject: pstore: Warn on PSTORE_TYPE_PMSG using deprecated function PMSG now uses ramoops_pstore_write_buf_user() instead of ...write_buf(). Print a ratelimited warning if gets accidentally called. Signed-off-by: Joel Fernandes [kees: adjusted commit log and added -EINVAL return] Signed-off-by: Kees Cook --- fs/pstore/ram.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'fs/pstore') diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index 6ad831b9d1b8..6d1393965b0a 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -288,10 +288,8 @@ static int notrace ramoops_pstore_write_buf(enum pstore_type_id type, persistent_ram_write(cxt->fprz, buf, size); return 0; } else if (type == PSTORE_TYPE_PMSG) { - if (!cxt->mprz) - return -ENOMEM; - persistent_ram_write(cxt->mprz, buf, size); - return 0; + pr_warn_ratelimited("PMSG shouldn't call %s\n", __func__); + return -EINVAL; } if (type != PSTORE_TYPE_DMESG) -- cgit v1.2.3 From 663deb47880f2283809669563c5a52ac7c6aef1a Mon Sep 17 00:00:00 2001 From: Joel Fernandes Date: Thu, 20 Oct 2016 00:34:01 -0700 Subject: pstore: Allow prz to control need for locking In preparation of not locking at all for certain buffers depending on if there's contention, make locking optional depending on the initialization of the prz. Signed-off-by: Joel Fernandes [kees: moved locking flag into prz instead of via caller arguments] Signed-off-by: Kees Cook --- fs/pstore/ram.c | 5 +++-- fs/pstore/ram_core.c | 24 +++++++++++++++--------- 2 files changed, 18 insertions(+), 11 deletions(-) (limited to 'fs/pstore') diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index 6d1393965b0a..86ac59066fba 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -432,7 +432,7 @@ static int ramoops_init_przs(struct device *dev, struct ramoops_context *cxt, for (i = 0; i < cxt->max_dump_cnt; i++) { cxt->przs[i] = persistent_ram_new(*paddr, cxt->record_size, 0, &cxt->ecc_info, - cxt->memtype); + cxt->memtype, 0); if (IS_ERR(cxt->przs[i])) { err = PTR_ERR(cxt->przs[i]); dev_err(dev, "failed to request mem region (0x%zx@0x%llx): %d\n", @@ -469,7 +469,8 @@ static int ramoops_init_prz(struct device *dev, struct ramoops_context *cxt, return -ENOMEM; } - *prz = persistent_ram_new(*paddr, sz, sig, &cxt->ecc_info, cxt->memtype); + *prz = persistent_ram_new(*paddr, sz, sig, &cxt->ecc_info, + cxt->memtype, 0); if (IS_ERR(*prz)) { int err = PTR_ERR(*prz); diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c index cb92055e6016..a857338b7dab 100644 --- a/fs/pstore/ram_core.c +++ b/fs/pstore/ram_core.c @@ -53,9 +53,10 @@ static size_t buffer_start_add(struct persistent_ram_zone *prz, size_t a) { int old; int new; - unsigned long flags; + unsigned long flags = 0; - raw_spin_lock_irqsave(&prz->buffer_lock, flags); + if (!(prz->flags & PRZ_FLAG_NO_LOCK)) + raw_spin_lock_irqsave(&prz->buffer_lock, flags); old = atomic_read(&prz->buffer->start); new = old + a; @@ -63,7 +64,8 @@ static size_t buffer_start_add(struct persistent_ram_zone *prz, size_t a) new -= prz->buffer_size; atomic_set(&prz->buffer->start, new); - raw_spin_unlock_irqrestore(&prz->buffer_lock, flags); + if (!(prz->flags & PRZ_FLAG_NO_LOCK)) + raw_spin_unlock_irqrestore(&prz->buffer_lock, flags); return old; } @@ -73,9 +75,10 @@ static void buffer_size_add(struct persistent_ram_zone *prz, size_t a) { size_t old; size_t new; - unsigned long flags; + unsigned long flags = 0; - raw_spin_lock_irqsave(&prz->buffer_lock, flags); + if (!(prz->flags & PRZ_FLAG_NO_LOCK)) + raw_spin_lock_irqsave(&prz->buffer_lock, flags); old = atomic_read(&prz->buffer->size); if (old == prz->buffer_size) @@ -87,7 +90,8 @@ static void buffer_size_add(struct persistent_ram_zone *prz, size_t a) atomic_set(&prz->buffer->size, new); exit: - raw_spin_unlock_irqrestore(&prz->buffer_lock, flags); + if (!(prz->flags & PRZ_FLAG_NO_LOCK)) + raw_spin_unlock_irqrestore(&prz->buffer_lock, flags); } static void notrace persistent_ram_encode_rs8(struct persistent_ram_zone *prz, @@ -463,7 +467,8 @@ static int persistent_ram_buffer_map(phys_addr_t start, phys_addr_t size, } static int persistent_ram_post_init(struct persistent_ram_zone *prz, u32 sig, - struct persistent_ram_ecc_info *ecc_info) + struct persistent_ram_ecc_info *ecc_info, + unsigned long flags) { int ret; @@ -492,6 +497,7 @@ static int persistent_ram_post_init(struct persistent_ram_zone *prz, u32 sig, prz->buffer->sig = sig; persistent_ram_zap(prz); prz->buffer_lock = __RAW_SPIN_LOCK_UNLOCKED(buffer_lock); + prz->flags = flags; return 0; } @@ -516,7 +522,7 @@ void persistent_ram_free(struct persistent_ram_zone *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) + unsigned int memtype, u32 flags) { struct persistent_ram_zone *prz; int ret = -ENOMEM; @@ -531,7 +537,7 @@ struct persistent_ram_zone *persistent_ram_new(phys_addr_t start, size_t size, if (ret) goto err; - ret = persistent_ram_post_init(prz, sig, ecc_info); + ret = persistent_ram_post_init(prz, sig, ecc_info, flags); if (ret) goto err; -- cgit v1.2.3 From de83209249d64bad993f25d3ea4bba57683e2e2e Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Fri, 11 Nov 2016 11:48:06 -0800 Subject: pstore: Make ramoops_init_przs generic for other prz arrays Currently ramoops_init_przs() is hard wired only for panic dump zone array. In preparation for the ftrace zone array (one zone per-cpu) and pmsg zone array, make the function more generic to be able to handle this case. Heavily based on similar work from Joel Fernandes. Signed-off-by: Kees Cook --- fs/pstore/ram.c | 82 +++++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 54 insertions(+), 28 deletions(-) (limited to 'fs/pstore') diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index 86ac59066fba..34294c32af0b 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -405,53 +405,78 @@ static void ramoops_free_przs(struct ramoops_context *cxt) } static int ramoops_init_przs(struct device *dev, struct ramoops_context *cxt, - phys_addr_t *paddr, size_t dump_mem_sz) + struct persistent_ram_zone ***przs, + phys_addr_t *paddr, size_t mem_sz, + ssize_t record_size, + unsigned int *cnt, u32 sig, u32 flags) { int err = -ENOMEM; int i; + size_t zone_sz; + struct persistent_ram_zone **prz_ar; - if (!cxt->record_size) + /* Allocate nothing for 0 mem_sz or 0 record_size. */ + if (mem_sz == 0 || record_size == 0) { + *cnt = 0; return 0; - - if (*paddr + dump_mem_sz - cxt->phys_addr > cxt->size) { - dev_err(dev, "no room for dumps\n"); - return -ENOMEM; } - cxt->max_dump_cnt = dump_mem_sz / cxt->record_size; - if (!cxt->max_dump_cnt) - return -ENOMEM; + /* + * If we have a negative record size, calculate it based on + * mem_sz / *cnt. If we have a positive record size, calculate + * cnt from mem_sz / record_size. + */ + if (record_size < 0) { + if (*cnt == 0) + return 0; + record_size = mem_sz / *cnt; + if (record_size == 0) + goto fail; + } else { + *cnt = mem_sz / record_size; + if (*cnt == 0) + goto fail; + } - cxt->przs = kzalloc(sizeof(*cxt->przs) * cxt->max_dump_cnt, - GFP_KERNEL); - if (!cxt->przs) { - dev_err(dev, "failed to initialize a prz array for dumps\n"); - goto fail_mem; + if (*paddr + mem_sz - cxt->phys_addr > cxt->size) { + dev_err(dev, "no room for mem region (0x%zx@0x%llx) in (0x%lx@0x%llx)\n", + mem_sz, (unsigned long long)*paddr, + cxt->size, (unsigned long long)cxt->phys_addr); + goto fail; } - for (i = 0; i < cxt->max_dump_cnt; i++) { - cxt->przs[i] = persistent_ram_new(*paddr, cxt->record_size, 0, + zone_sz = mem_sz / *cnt; + if (!zone_sz) + goto fail; + + prz_ar = kcalloc(*cnt, sizeof(**przs), GFP_KERNEL); + if (!prz_ar) + goto fail; + + for (i = 0; i < *cnt; i++) { + prz_ar[i] = persistent_ram_new(*paddr, zone_sz, sig, &cxt->ecc_info, - cxt->memtype, 0); - if (IS_ERR(cxt->przs[i])) { - err = PTR_ERR(cxt->przs[i]); + cxt->memtype, flags); + if (IS_ERR(prz_ar[i])) { + err = PTR_ERR(prz_ar[i]); dev_err(dev, "failed to request mem region (0x%zx@0x%llx): %d\n", - cxt->record_size, (unsigned long long)*paddr, err); + record_size, (unsigned long long)*paddr, err); while (i > 0) { i--; - persistent_ram_free(cxt->przs[i]); + persistent_ram_free(prz_ar[i]); } - goto fail_prz; + kfree(prz_ar); + goto fail; } - *paddr += cxt->record_size; + *paddr += zone_sz; } + *przs = prz_ar; return 0; -fail_prz: - kfree(cxt->przs); -fail_mem: - cxt->max_dump_cnt = 0; + +fail: + *cnt = 0; return err; } @@ -605,7 +630,8 @@ static int ramoops_probe(struct platform_device *pdev) dump_mem_sz = cxt->size - cxt->console_size - cxt->ftrace_size - cxt->pmsg_size; - err = ramoops_init_przs(dev, cxt, &paddr, dump_mem_sz); + err = ramoops_init_przs(dev, cxt, &cxt->przs, &paddr, dump_mem_sz, + cxt->record_size, &cxt->max_dump_cnt, 0, 0); if (err) goto fail_out; -- cgit v1.2.3 From a1cf53ac6d156721afa86453d5e8423461881231 Mon Sep 17 00:00:00 2001 From: Joel Fernandes Date: Thu, 20 Oct 2016 00:34:04 -0700 Subject: ramoops: Split ftrace buffer space into per-CPU zones If the RAMOOPS_FLAG_FTRACE_PER_CPU flag is passed to ramoops pdata, split the ftrace space into multiple zones depending on the number of CPUs. This speeds up the performance of function tracing by about 280% in my tests as we avoid the locking. The trade off being lesser space available per CPU. Let the ramoops user decide which option they want based on pdata flag. Signed-off-by: Joel Fernandes [kees: added max_ftrace_cnt to track size, added DT logic and docs] Signed-off-by: Kees Cook --- fs/pstore/ram.c | 72 +++++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 55 insertions(+), 17 deletions(-) (limited to 'fs/pstore') diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index 34294c32af0b..0ed3fec04c9b 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -87,7 +87,7 @@ MODULE_PARM_DESC(ramoops_ecc, struct ramoops_context { struct persistent_ram_zone **przs; struct persistent_ram_zone *cprz; - struct persistent_ram_zone *fprz; + struct persistent_ram_zone **fprzs; struct persistent_ram_zone *mprz; phys_addr_t phys_addr; unsigned long size; @@ -97,12 +97,14 @@ struct ramoops_context { size_t ftrace_size; size_t pmsg_size; int dump_oops; + u32 flags; struct persistent_ram_ecc_info ecc_info; unsigned int max_dump_cnt; unsigned int dump_write_cnt; /* _read_cnt need clear on ramoops_pstore_open */ unsigned int dump_read_cnt; unsigned int console_read_cnt; + unsigned int max_ftrace_cnt; unsigned int ftrace_read_cnt; unsigned int pmsg_read_cnt; struct pstore_info pstore; @@ -219,9 +221,17 @@ static ssize_t ramoops_pstore_read(u64 *id, enum pstore_type_id *type, if (!prz_ok(prz)) prz = ramoops_get_next_prz(&cxt->cprz, &cxt->console_read_cnt, 1, id, type, PSTORE_TYPE_CONSOLE, 0); - if (!prz_ok(prz)) - prz = ramoops_get_next_prz(&cxt->fprz, &cxt->ftrace_read_cnt, - 1, id, type, PSTORE_TYPE_FTRACE, 0); + if (!prz_ok(prz)) { + while (cxt->ftrace_read_cnt < cxt->max_ftrace_cnt && !prz) { + prz = ramoops_get_next_prz(cxt->fprzs, + &cxt->ftrace_read_cnt, + cxt->max_ftrace_cnt, id, type, + PSTORE_TYPE_FTRACE, 0); + if (!prz_ok(prz)) + continue; + } + } + if (!prz_ok(prz)) prz = ramoops_get_next_prz(&cxt->mprz, &cxt->pmsg_read_cnt, 1, id, type, PSTORE_TYPE_PMSG, 0); @@ -283,9 +293,19 @@ static int notrace ramoops_pstore_write_buf(enum pstore_type_id type, persistent_ram_write(cxt->cprz, buf, size); return 0; } else if (type == PSTORE_TYPE_FTRACE) { - if (!cxt->fprz) + int zonenum; + + if (!cxt->fprzs) return -ENOMEM; - persistent_ram_write(cxt->fprz, buf, size); + /* + * Choose zone by if we're using per-cpu buffers. + */ + if (cxt->flags & RAMOOPS_FLAG_FTRACE_PER_CPU) + zonenum = smp_processor_id(); + else + zonenum = 0; + + persistent_ram_write(cxt->fprzs[zonenum], buf, size); return 0; } else if (type == PSTORE_TYPE_PMSG) { pr_warn_ratelimited("PMSG shouldn't call %s\n", __func__); @@ -363,7 +383,9 @@ static int ramoops_pstore_erase(enum pstore_type_id type, u64 id, int count, prz = cxt->cprz; break; case PSTORE_TYPE_FTRACE: - prz = cxt->fprz; + if (id >= cxt->max_ftrace_cnt) + return -EINVAL; + prz = cxt->fprzs[id]; break; case PSTORE_TYPE_PMSG: prz = cxt->mprz; @@ -394,14 +416,22 @@ static void ramoops_free_przs(struct ramoops_context *cxt) { int i; - if (!cxt->przs) - return; + /* Free dump PRZs */ + if (cxt->przs) { + for (i = 0; i < cxt->max_dump_cnt; i++) + persistent_ram_free(cxt->przs[i]); - for (i = 0; i < cxt->max_dump_cnt; i++) - persistent_ram_free(cxt->przs[i]); + kfree(cxt->przs); + cxt->max_dump_cnt = 0; + } - kfree(cxt->przs); - cxt->max_dump_cnt = 0; + /* Free ftrace PRZs */ + if (cxt->fprzs) { + for (i = 0; i < cxt->max_ftrace_cnt; i++) + persistent_ram_free(cxt->fprzs[i]); + kfree(cxt->fprzs); + cxt->max_ftrace_cnt = 0; + } } static int ramoops_init_przs(struct device *dev, struct ramoops_context *cxt, @@ -567,6 +597,7 @@ static int ramoops_parse_dt(struct platform_device *pdev, parse_size("ftrace-size", pdata->ftrace_size); parse_size("pmsg-size", pdata->pmsg_size); parse_size("ecc-size", pdata->ecc_info.ecc_size); + parse_size("flags", pdata->flags); #undef parse_size @@ -624,6 +655,7 @@ static int ramoops_probe(struct platform_device *pdev) cxt->ftrace_size = pdata->ftrace_size; cxt->pmsg_size = pdata->pmsg_size; cxt->dump_oops = pdata->dump_oops; + cxt->flags = pdata->flags; cxt->ecc_info = pdata->ecc_info; paddr = cxt->phys_addr; @@ -640,8 +672,14 @@ static int ramoops_probe(struct platform_device *pdev) if (err) goto fail_init_cprz; - err = ramoops_init_prz(dev, cxt, &cxt->fprz, &paddr, cxt->ftrace_size, - LINUX_VERSION_CODE); + cxt->max_ftrace_cnt = (cxt->flags & RAMOOPS_FLAG_FTRACE_PER_CPU) + ? nr_cpu_ids + : 1; + err = ramoops_init_przs(dev, cxt, &cxt->fprzs, &paddr, cxt->ftrace_size, + -1, &cxt->max_ftrace_cnt, + LINUX_VERSION_CODE, + (cxt->flags & RAMOOPS_FLAG_FTRACE_PER_CPU) + ? PRZ_FLAG_NO_LOCK : 0); if (err) goto fail_init_fprz; @@ -705,7 +743,6 @@ fail_clear: cxt->pstore.bufsize = 0; persistent_ram_free(cxt->mprz); fail_init_mprz: - persistent_ram_free(cxt->fprz); fail_init_fprz: persistent_ram_free(cxt->cprz); fail_init_cprz: @@ -724,7 +761,6 @@ static int ramoops_remove(struct platform_device *pdev) cxt->pstore.bufsize = 0; persistent_ram_free(cxt->mprz); - persistent_ram_free(cxt->fprz); persistent_ram_free(cxt->cprz); ramoops_free_przs(cxt); @@ -766,6 +802,8 @@ static void ramoops_register_dummy(void) dummy_data->ftrace_size = ramoops_ftrace_size; dummy_data->pmsg_size = ramoops_pmsg_size; dummy_data->dump_oops = dump_oops; + dummy_data->flags = RAMOOPS_FLAG_FTRACE_PER_CPU; + /* * For backwards compatibility ramoops.ecc=1 means 16 bytes ECC * (using 1 byte for ECC isn't much of use anyway). -- cgit v1.2.3 From fbccdeb8d77d6830556bc4079eeed80298cc97dc Mon Sep 17 00:00:00 2001 From: Joel Fernandes Date: Thu, 20 Oct 2016 00:34:05 -0700 Subject: pstore: Add ftrace timestamp counter In preparation for merging the per CPU buffers into one buffer when we retrieve the pstore ftrace data, we store the timestamp as a counter in the ftrace pstore record. We store the CPU number as well if !PSTORE_CPU_IN_IP, in this case we shift the counter and may lose ordering there but we preserve the same record size. The timestamp counter is also racy, and not doing any locking or synchronization here results in the benefit of lower overhead. Since we don't care much here for exact ordering of function traces across CPUs, we don't synchronize and may lose some counter updates but I'm ok with that. Using trace_clock() results in much lower performance so avoid using it since we don't want accuracy in timestamp and need a rough ordering to perform merge. Signed-off-by: Joel Fernandes [kees: updated commit message, added comments] Signed-off-by: Kees Cook --- fs/pstore/ftrace.c | 4 ++++ fs/pstore/inode.c | 8 +++++--- fs/pstore/internal.h | 34 ---------------------------------- 3 files changed, 9 insertions(+), 37 deletions(-) (limited to 'fs/pstore') diff --git a/fs/pstore/ftrace.c b/fs/pstore/ftrace.c index d4887705bb61..31548cc09e7b 100644 --- a/fs/pstore/ftrace.c +++ b/fs/pstore/ftrace.c @@ -27,6 +27,9 @@ #include #include "internal.h" +/* This doesn't need to be atomic: speed is chosen over correctness here. */ +static u64 pstore_ftrace_stamp; + static void notrace pstore_ftrace_call(unsigned long ip, unsigned long parent_ip, struct ftrace_ops *op, @@ -42,6 +45,7 @@ static void notrace pstore_ftrace_call(unsigned long ip, rec.ip = ip; rec.parent_ip = parent_ip; + pstore_ftrace_write_timestamp(&rec, pstore_ftrace_stamp++); pstore_ftrace_encode_cpu(&rec, raw_smp_processor_id()); psinfo->write_buf(PSTORE_TYPE_FTRACE, 0, NULL, 0, (void *)&rec, 0, sizeof(rec), psinfo); diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c index 1781dc50762e..0d6bbcf47d52 100644 --- a/fs/pstore/inode.c +++ b/fs/pstore/inode.c @@ -107,9 +107,11 @@ static int pstore_ftrace_seq_show(struct seq_file *s, void *v) struct pstore_ftrace_seq_data *data = v; struct pstore_ftrace_record *rec = (void *)(ps->data + data->off); - seq_printf(s, "%d %08lx %08lx %pf <- %pF\n", - pstore_ftrace_decode_cpu(rec), rec->ip, rec->parent_ip, - (void *)rec->ip, (void *)rec->parent_ip); + seq_printf(s, "CPU:%d ts:%llu %08lx %08lx %pf <- %pF\n", + pstore_ftrace_decode_cpu(rec), + pstore_ftrace_read_timestamp(rec), + rec->ip, rec->parent_ip, (void *)rec->ip, + (void *)rec->parent_ip); return 0; } diff --git a/fs/pstore/internal.h b/fs/pstore/internal.h index e38a22b31282..da416e6591c9 100644 --- a/fs/pstore/internal.h +++ b/fs/pstore/internal.h @@ -5,40 +5,6 @@ #include #include -#if NR_CPUS <= 2 && defined(CONFIG_ARM_THUMB) -#define PSTORE_CPU_IN_IP 0x1 -#elif NR_CPUS <= 4 && defined(CONFIG_ARM) -#define PSTORE_CPU_IN_IP 0x3 -#endif - -struct pstore_ftrace_record { - unsigned long ip; - unsigned long parent_ip; -#ifndef PSTORE_CPU_IN_IP - unsigned int cpu; -#endif -}; - -static inline void -pstore_ftrace_encode_cpu(struct pstore_ftrace_record *rec, unsigned int cpu) -{ -#ifndef PSTORE_CPU_IN_IP - rec->cpu = cpu; -#else - rec->ip |= cpu; -#endif -} - -static inline unsigned int -pstore_ftrace_decode_cpu(struct pstore_ftrace_record *rec) -{ -#ifndef PSTORE_CPU_IN_IP - return rec->cpu; -#else - return rec->ip & PSTORE_CPU_IN_IP; -#endif -} - #ifdef CONFIG_PSTORE_FTRACE extern void pstore_register_ftrace(void); extern void pstore_unregister_ftrace(void); -- cgit v1.2.3 From 2fbea82bbb893e07a6f5c23397a60ec9933e54b7 Mon Sep 17 00:00:00 2001 From: Joel Fernandes Date: Thu, 20 Oct 2016 00:34:06 -0700 Subject: pstore: Merge per-CPU ftrace records into one Up until this patch, each of the per CPU ftrace buffers appear as a separate ftrace-ramoops-N file. In this patch we merge all the zones into one and populate a single ftrace-ramoops-0 file. Signed-off-by: Joel Fernandes [kees: clarified variables names, added -ENOMEM handling] Signed-off-by: Kees Cook --- fs/pstore/ram.c | 122 ++++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 109 insertions(+), 13 deletions(-) (limited to 'fs/pstore') diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index 0ed3fec04c9b..f5d266157964 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -182,16 +182,69 @@ static bool prz_ok(struct persistent_ram_zone *prz) persistent_ram_ecc_string(prz, NULL, 0)); } +static ssize_t ftrace_log_combine(struct persistent_ram_zone *dest, + struct persistent_ram_zone *src) +{ + size_t dest_size, src_size, total, dest_off, src_off; + size_t dest_idx = 0, src_idx = 0, merged_idx = 0; + void *merged_buf; + struct pstore_ftrace_record *drec, *srec, *mrec; + size_t record_size = sizeof(struct pstore_ftrace_record); + + dest_off = dest->old_log_size % record_size; + dest_size = dest->old_log_size - dest_off; + + src_off = src->old_log_size % record_size; + src_size = src->old_log_size - src_off; + + total = dest_size + src_size; + merged_buf = kmalloc(total, GFP_KERNEL); + if (!merged_buf) + return -ENOMEM; + + drec = (struct pstore_ftrace_record *)(dest->old_log + dest_off); + srec = (struct pstore_ftrace_record *)(src->old_log + src_off); + mrec = (struct pstore_ftrace_record *)(merged_buf); + + while (dest_size > 0 && src_size > 0) { + if (pstore_ftrace_read_timestamp(&drec[dest_idx]) < + pstore_ftrace_read_timestamp(&srec[src_idx])) { + mrec[merged_idx++] = drec[dest_idx++]; + dest_size -= record_size; + } else { + mrec[merged_idx++] = srec[src_idx++]; + src_size -= record_size; + } + } + + while (dest_size > 0) { + mrec[merged_idx++] = drec[dest_idx++]; + dest_size -= record_size; + } + + while (src_size > 0) { + mrec[merged_idx++] = srec[src_idx++]; + src_size -= record_size; + } + + kfree(dest->old_log); + dest->old_log = merged_buf; + dest->old_log_size = total; + + return 0; +} + static ssize_t ramoops_pstore_read(u64 *id, enum pstore_type_id *type, int *count, struct timespec *time, char **buf, bool *compressed, ssize_t *ecc_notice_size, struct pstore_info *psi) { - ssize_t size; + ssize_t size = 0; struct ramoops_context *cxt = psi->data; struct persistent_ram_zone *prz = NULL; int header_length = 0; + bool free_prz = false; /* Ramoops headers provide time stamps for PSTORE_TYPE_DMESG, but * PSTORE_TYPE_CONSOLE and PSTORE_TYPE_FTRACE don't currently have @@ -221,22 +274,56 @@ static ssize_t ramoops_pstore_read(u64 *id, enum pstore_type_id *type, if (!prz_ok(prz)) prz = ramoops_get_next_prz(&cxt->cprz, &cxt->console_read_cnt, 1, id, type, PSTORE_TYPE_CONSOLE, 0); + + if (!prz_ok(prz)) + prz = ramoops_get_next_prz(&cxt->mprz, &cxt->pmsg_read_cnt, + 1, id, type, PSTORE_TYPE_PMSG, 0); + + /* ftrace is last since it may want to dynamically allocate memory. */ if (!prz_ok(prz)) { - while (cxt->ftrace_read_cnt < cxt->max_ftrace_cnt && !prz) { + if (!(cxt->flags & RAMOOPS_FLAG_FTRACE_PER_CPU)) { prz = ramoops_get_next_prz(cxt->fprzs, - &cxt->ftrace_read_cnt, - cxt->max_ftrace_cnt, id, type, + &cxt->ftrace_read_cnt, 1, id, type, PSTORE_TYPE_FTRACE, 0); - if (!prz_ok(prz)) - continue; + } else { + /* + * Build a new dummy record which combines all the + * per-cpu records including metadata and ecc info. + */ + struct persistent_ram_zone *tmp_prz, *prz_next; + + tmp_prz = kzalloc(sizeof(struct persistent_ram_zone), + GFP_KERNEL); + if (!tmp_prz) + return -ENOMEM; + free_prz = true; + + while (cxt->ftrace_read_cnt < cxt->max_ftrace_cnt) { + prz_next = ramoops_get_next_prz(cxt->fprzs, + &cxt->ftrace_read_cnt, + cxt->max_ftrace_cnt, id, + type, PSTORE_TYPE_FTRACE, 0); + + if (!prz_ok(prz_next)) + continue; + + tmp_prz->ecc_info = prz_next->ecc_info; + tmp_prz->corrected_bytes += + prz_next->corrected_bytes; + tmp_prz->bad_blocks += prz_next->bad_blocks; + size = ftrace_log_combine(tmp_prz, prz_next); + if (size) + goto out; + } + *id = 0; + prz = tmp_prz; } } - if (!prz_ok(prz)) - prz = ramoops_get_next_prz(&cxt->mprz, &cxt->pmsg_read_cnt, - 1, id, type, PSTORE_TYPE_PMSG, 0); - if (!prz_ok(prz)) - return 0; + if (!prz_ok(prz)) { + size = 0; + goto out; + } size = persistent_ram_old_size(prz) - header_length; @@ -244,12 +331,21 @@ static ssize_t ramoops_pstore_read(u64 *id, enum pstore_type_id *type, *ecc_notice_size = persistent_ram_ecc_string(prz, NULL, 0); *buf = kmalloc(size + *ecc_notice_size + 1, GFP_KERNEL); - if (*buf == NULL) - return -ENOMEM; + if (*buf == NULL) { + size = -ENOMEM; + goto out; + } memcpy(*buf, (char *)persistent_ram_old(prz) + header_length, size); + persistent_ram_ecc_string(prz, *buf + size, *ecc_notice_size + 1); +out: + if (free_prz) { + kfree(prz->old_log); + kfree(prz); + } + return size; } -- cgit v1.2.3 From c443a5f3f1f1f5d49f00537bf8cca7aa86a4aac8 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Fri, 11 Nov 2016 14:05:57 -0800 Subject: pstore: improve error report for failed setup When setting ramoops record sizes, sometimes it's not clear which parameters contributed to the allocation failure. This adds a per-zone name and expands the failure reports. Signed-off-by: Kees Cook --- fs/pstore/ram.c | 53 ++++++++++++++++++++++++++++++++++------------------- 1 file changed, 34 insertions(+), 19 deletions(-) (limited to 'fs/pstore') diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index f5d266157964..64fd6ead82cc 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -530,7 +530,8 @@ static void ramoops_free_przs(struct ramoops_context *cxt) } } -static int ramoops_init_przs(struct device *dev, struct ramoops_context *cxt, +static int ramoops_init_przs(const char *name, + struct device *dev, struct ramoops_context *cxt, struct persistent_ram_zone ***przs, phys_addr_t *paddr, size_t mem_sz, ssize_t record_size, @@ -556,24 +557,33 @@ static int ramoops_init_przs(struct device *dev, struct ramoops_context *cxt, if (*cnt == 0) return 0; record_size = mem_sz / *cnt; - if (record_size == 0) + if (record_size == 0) { + dev_err(dev, "%s record size == 0 (%zu / %u)\n", + name, mem_sz, *cnt); goto fail; + } } else { *cnt = mem_sz / record_size; - if (*cnt == 0) + if (*cnt == 0) { + dev_err(dev, "%s record count == 0 (%zu / %zu)\n", + name, mem_sz, record_size); goto fail; + } } if (*paddr + mem_sz - cxt->phys_addr > cxt->size) { - dev_err(dev, "no room for mem region (0x%zx@0x%llx) in (0x%lx@0x%llx)\n", + dev_err(dev, "no room for %s mem region (0x%zx@0x%llx) in (0x%lx@0x%llx)\n", + name, mem_sz, (unsigned long long)*paddr, cxt->size, (unsigned long long)cxt->phys_addr); goto fail; } zone_sz = mem_sz / *cnt; - if (!zone_sz) + if (!zone_sz) { + dev_err(dev, "%s zone size == 0\n", name); goto fail; + } prz_ar = kcalloc(*cnt, sizeof(**przs), GFP_KERNEL); if (!prz_ar) @@ -585,8 +595,9 @@ static int ramoops_init_przs(struct device *dev, struct ramoops_context *cxt, cxt->memtype, flags); if (IS_ERR(prz_ar[i])) { err = PTR_ERR(prz_ar[i]); - dev_err(dev, "failed to request mem region (0x%zx@0x%llx): %d\n", - record_size, (unsigned long long)*paddr, err); + dev_err(dev, "failed to request %s mem region (0x%zx@0x%llx): %d\n", + name, record_size, + (unsigned long long)*paddr, err); while (i > 0) { i--; @@ -606,7 +617,8 @@ fail: return err; } -static int ramoops_init_prz(struct device *dev, struct ramoops_context *cxt, +static int ramoops_init_prz(const char *name, + struct device *dev, struct ramoops_context *cxt, struct persistent_ram_zone **prz, phys_addr_t *paddr, size_t sz, u32 sig) { @@ -614,8 +626,8 @@ static int ramoops_init_prz(struct device *dev, struct ramoops_context *cxt, return 0; if (*paddr + sz - cxt->phys_addr > cxt->size) { - dev_err(dev, "no room for mem region (0x%zx@0x%llx) in (0x%lx@0x%llx)\n", - sz, (unsigned long long)*paddr, + dev_err(dev, "no room for %s mem region (0x%zx@0x%llx) in (0x%lx@0x%llx)\n", + name, sz, (unsigned long long)*paddr, cxt->size, (unsigned long long)cxt->phys_addr); return -ENOMEM; } @@ -625,8 +637,8 @@ static int ramoops_init_prz(struct device *dev, struct ramoops_context *cxt, if (IS_ERR(*prz)) { int err = PTR_ERR(*prz); - dev_err(dev, "failed to request mem region (0x%zx@0x%llx): %d\n", - sz, (unsigned long long)*paddr, err); + dev_err(dev, "failed to request %s mem region (0x%zx@0x%llx): %d\n", + name, sz, (unsigned long long)*paddr, err); return err; } @@ -712,6 +724,7 @@ static int ramoops_probe(struct platform_device *pdev) if (dev_of_node(dev) && !pdata) { pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL); if (!pdata) { + pr_err("cannot allocate platform data buffer\n"); err = -ENOMEM; goto fail_out; } @@ -758,12 +771,13 @@ static int ramoops_probe(struct platform_device *pdev) dump_mem_sz = cxt->size - cxt->console_size - cxt->ftrace_size - cxt->pmsg_size; - err = ramoops_init_przs(dev, cxt, &cxt->przs, &paddr, dump_mem_sz, - cxt->record_size, &cxt->max_dump_cnt, 0, 0); + err = ramoops_init_przs("dump", dev, cxt, &cxt->przs, &paddr, + dump_mem_sz, cxt->record_size, + &cxt->max_dump_cnt, 0, 0); if (err) goto fail_out; - err = ramoops_init_prz(dev, cxt, &cxt->cprz, &paddr, + err = ramoops_init_prz("console", dev, cxt, &cxt->cprz, &paddr, cxt->console_size, 0); if (err) goto fail_init_cprz; @@ -771,15 +785,16 @@ static int ramoops_probe(struct platform_device *pdev) cxt->max_ftrace_cnt = (cxt->flags & RAMOOPS_FLAG_FTRACE_PER_CPU) ? nr_cpu_ids : 1; - err = ramoops_init_przs(dev, cxt, &cxt->fprzs, &paddr, cxt->ftrace_size, - -1, &cxt->max_ftrace_cnt, - LINUX_VERSION_CODE, + err = ramoops_init_przs("ftrace", dev, cxt, &cxt->fprzs, &paddr, + cxt->ftrace_size, -1, + &cxt->max_ftrace_cnt, LINUX_VERSION_CODE, (cxt->flags & RAMOOPS_FLAG_FTRACE_PER_CPU) ? PRZ_FLAG_NO_LOCK : 0); if (err) goto fail_init_fprz; - err = ramoops_init_prz(dev, cxt, &cxt->mprz, &paddr, cxt->pmsg_size, 0); + err = ramoops_init_prz("pmsg", dev, cxt, &cxt->mprz, &paddr, + cxt->pmsg_size, 0); if (err) goto fail_init_mprz; -- cgit v1.2.3 From a5d23b956cabd5931d4aff07573cd9d053bacd74 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Fri, 11 Nov 2016 14:30:11 -0800 Subject: pstore: Clarify context field przs as dprzs Since "przs" (persistent ram zones) is a general name in the code now, so rename the Oops-dump zones to dprzs from przs. Based on a patch from Nobuhiro Iwamatsu. Signed-off-by: Kees Cook --- fs/pstore/ram.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) (limited to 'fs/pstore') diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index 64fd6ead82cc..380222432eff 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -85,10 +85,10 @@ MODULE_PARM_DESC(ramoops_ecc, "bytes ECC)"); struct ramoops_context { - struct persistent_ram_zone **przs; - struct persistent_ram_zone *cprz; - struct persistent_ram_zone **fprzs; - struct persistent_ram_zone *mprz; + struct persistent_ram_zone **dprzs; /* Oops dump zones */ + struct persistent_ram_zone *cprz; /* Console zone */ + struct persistent_ram_zone **fprzs; /* Ftrace zones */ + struct persistent_ram_zone *mprz; /* PMSG zone */ phys_addr_t phys_addr; unsigned long size; unsigned int memtype; @@ -256,7 +256,7 @@ static ssize_t ramoops_pstore_read(u64 *id, enum pstore_type_id *type, /* Find the next valid persistent_ram_zone for DMESG */ while (cxt->dump_read_cnt < cxt->max_dump_cnt && !prz) { - prz = ramoops_get_next_prz(cxt->przs, &cxt->dump_read_cnt, + prz = ramoops_get_next_prz(cxt->dprzs, &cxt->dump_read_cnt, cxt->max_dump_cnt, id, type, PSTORE_TYPE_DMESG, 1); if (!prz_ok(prz)) @@ -430,10 +430,10 @@ static int notrace ramoops_pstore_write_buf(enum pstore_type_id type, if (part != 1) return -ENOSPC; - if (!cxt->przs) + if (!cxt->dprzs) return -ENOSPC; - prz = cxt->przs[cxt->dump_write_cnt]; + prz = cxt->dprzs[cxt->dump_write_cnt]; hlen = ramoops_write_kmsg_hdr(prz, compressed); if (size + hlen > prz->buffer_size) @@ -473,7 +473,7 @@ static int ramoops_pstore_erase(enum pstore_type_id type, u64 id, int count, case PSTORE_TYPE_DMESG: if (id >= cxt->max_dump_cnt) return -EINVAL; - prz = cxt->przs[id]; + prz = cxt->dprzs[id]; break; case PSTORE_TYPE_CONSOLE: prz = cxt->cprz; @@ -513,11 +513,11 @@ static void ramoops_free_przs(struct ramoops_context *cxt) int i; /* Free dump PRZs */ - if (cxt->przs) { + if (cxt->dprzs) { for (i = 0; i < cxt->max_dump_cnt; i++) - persistent_ram_free(cxt->przs[i]); + persistent_ram_free(cxt->dprzs[i]); - kfree(cxt->przs); + kfree(cxt->dprzs); cxt->max_dump_cnt = 0; } @@ -771,7 +771,7 @@ static int ramoops_probe(struct platform_device *pdev) dump_mem_sz = cxt->size - cxt->console_size - cxt->ftrace_size - cxt->pmsg_size; - err = ramoops_init_przs("dump", dev, cxt, &cxt->przs, &paddr, + err = ramoops_init_przs("dump", dev, cxt, &cxt->dprzs, &paddr, dump_mem_sz, cxt->record_size, &cxt->max_dump_cnt, 0, 0); if (err) -- cgit v1.2.3 From 7a0032f50472c740e35e849366572c124087a346 Mon Sep 17 00:00:00 2001 From: Joel Fernandes Date: Tue, 15 Nov 2016 12:31:21 -0800 Subject: pstore: Use global ftrace filters for function trace filtering Currently, pstore doesn't have any filters setup for function tracing. This has the associated overhead and may not be useful for users looking for tracing specific set of functions. ftrace's regular function trace filtering is done writing to tracing/set_ftrace_filter however this is not available if not requested. In order to be able to use this feature, the support to request global filtering introduced earlier in the series should be requested before registering the ftrace ops. Here we do the same. Signed-off-by: Joel Fernandes Signed-off-by: Kees Cook --- fs/pstore/ftrace.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'fs/pstore') diff --git a/fs/pstore/ftrace.c b/fs/pstore/ftrace.c index 31548cc09e7b..899d0ba0bd6c 100644 --- a/fs/pstore/ftrace.c +++ b/fs/pstore/ftrace.c @@ -75,10 +75,13 @@ static ssize_t pstore_ftrace_knob_write(struct file *f, const char __user *buf, if (!on ^ pstore_ftrace_enabled) goto out; - if (on) + if (on) { + ftrace_ops_set_global_filter(&pstore_ftrace_ops); ret = register_ftrace_function(&pstore_ftrace_ops); - else + } else { ret = unregister_ftrace_function(&pstore_ftrace_ops); + } + if (ret) { pr_err("%s: unable to %sregister ftrace ops: %zd\n", __func__, on ? "" : "un", ret); -- cgit v1.2.3 From e9e360b08a44098ec6f31de8e5a29a3ffaada828 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Wed, 19 Oct 2016 10:23:40 +0900 Subject: pstore: Protect unlink with read_mutex When update_ms is set, pstore_get_records() will be called when there's a new entry. But unlink can be called at the same time and might contend with the open-read-close loop. Depending on the implementation of platform driver, it may be safe or not. But I think it'd be better to protect those race in the first place. Cc: Stefan Hajnoczi Signed-off-by: Namhyung Kim Signed-off-by: Kees Cook --- fs/pstore/inode.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'fs/pstore') diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c index 0d6bbcf47d52..57c0646479f5 100644 --- a/fs/pstore/inode.c +++ b/fs/pstore/inode.c @@ -199,11 +199,14 @@ static int pstore_unlink(struct inode *dir, struct dentry *dentry) if (err) return err; - if (p->psi->erase) + if (p->psi->erase) { + mutex_lock(&p->psi->read_mutex); p->psi->erase(p->type, p->id, p->count, d_inode(dentry)->i_ctime, p->psi); - else + mutex_unlock(&p->psi->read_mutex); + } else { return -EPERM; + } return simple_unlink(dir, dentry); } -- cgit v1.2.3 From 70ad35db3321a6d129245979de4ac9d06eed897c Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Wed, 19 Oct 2016 10:23:41 +0900 Subject: pstore: Convert console write to use ->write_buf Maybe I'm missing something, but I don't know why it needs to copy the input buffer to psinfo->buf and then write. Instead we can write the input buffer directly. The only implementation that supports console message (i.e. ramoops) already does it for ftrace messages. For the upcoming virtio backend driver, it needs to protect psinfo->buf overwritten from console messages. If it could use ->write_buf method instead of ->write, the problem will be solved easily. Cc: Stefan Hajnoczi Signed-off-by: Namhyung Kim Signed-off-by: Kees Cook --- fs/pstore/platform.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'fs/pstore') diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c index 60e6db6f5da2..729677e18e36 100644 --- a/fs/pstore/platform.c +++ b/fs/pstore/platform.c @@ -585,8 +585,8 @@ static void pstore_console_write(struct console *con, const char *s, unsigned c) } else { spin_lock_irqsave(&psinfo->buf_lock, flags); } - memcpy(psinfo->buf, s, c); - psinfo->write(PSTORE_TYPE_CONSOLE, 0, &id, 0, 0, 0, c, psinfo); + psinfo->write_buf(PSTORE_TYPE_CONSOLE, 0, &id, 0, + s, 0, c, psinfo); spin_unlock_irqrestore(&psinfo->buf_lock, flags); s += c; c = e - s; -- cgit v1.2.3 From fc46d4e453f50d2b376267f180cae250b54f9fb4 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Tue, 15 Nov 2016 16:29:40 -0800 Subject: ramoops: add pdata NULL check to ramoops_probe This adds a check for a NULL platform data, which should only be possible if a driver incorrectly sets up a probe request without also having defined the platform_data structure. This is based on a patch from Geliang Tang. Signed-off-by: Kees Cook --- fs/pstore/ram.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) (limited to 'fs/pstore') diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index 380222432eff..27c059e1760a 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -734,11 +734,20 @@ static int ramoops_probe(struct platform_device *pdev) goto fail_out; } - /* Only a single ramoops area allowed at a time, so fail extra + /* + * Only a single ramoops area allowed at a time, so fail extra * probes. */ - if (cxt->max_dump_cnt) + if (cxt->max_dump_cnt) { + pr_err("already initialized\n"); goto fail_out; + } + + /* Make sure we didn't get bogus platform data pointer. */ + if (!pdata) { + pr_err("NULL platform data\n"); + goto fail_out; + } if (!pdata->mem_size || (!pdata->record_size && !pdata->console_size && !pdata->ftrace_size && !pdata->pmsg_size)) { -- cgit v1.2.3