From 182ca6e0ae23550cf98d05daeab8dcb7fdc0a928 Mon Sep 17 00:00:00 2001 From: Yue Hu Date: Mon, 21 Jan 2019 18:20:41 +0800 Subject: pstore/ram: Replace dummy_data heap memory with stack memory In ramoops_register_dummy() dummy_data is allocated via kzalloc() then it will always occupy the heap space after register platform device via platform_device_register_data(), but it will not be used any more. So let's free it for system usage, replace it with stack memory is better due to small size. Signed-off-by: Yue Hu [kees: add required memset and adjust sizeof() argument] Signed-off-by: Kees Cook --- fs/pstore/ram.c | 35 ++++++++++++++--------------------- 1 file changed, 14 insertions(+), 21 deletions(-) diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index 898c8321b343..1adb5e35c004 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -110,7 +110,6 @@ struct ramoops_context { }; static struct platform_device *dummy; -static struct ramoops_platform_data *dummy_data; static int ramoops_pstore_open(struct pstore_info *psi) { @@ -892,13 +891,12 @@ static inline void ramoops_unregister_dummy(void) { platform_device_unregister(dummy); dummy = NULL; - - kfree(dummy_data); - dummy_data = NULL; } static void __init ramoops_register_dummy(void) { + struct ramoops_platform_data pdata; + /* * Prepare a dummy platform data structure to carry the module * parameters. If mem_size isn't set, then there are no module @@ -909,30 +907,25 @@ static void __init ramoops_register_dummy(void) pr_info("using module parameters\n"); - dummy_data = kzalloc(sizeof(*dummy_data), GFP_KERNEL); - if (!dummy_data) { - pr_info("could not allocate pdata\n"); - return; - } - - dummy_data->mem_size = mem_size; - dummy_data->mem_address = mem_address; - dummy_data->mem_type = mem_type; - dummy_data->record_size = record_size; - dummy_data->console_size = ramoops_console_size; - 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; + memset(&pdata, 0, sizeof(pdata)); + pdata.mem_size = mem_size; + pdata.mem_address = mem_address; + pdata.mem_type = mem_type; + pdata.record_size = record_size; + pdata.console_size = ramoops_console_size; + pdata.ftrace_size = ramoops_ftrace_size; + pdata.pmsg_size = ramoops_pmsg_size; + pdata.dump_oops = dump_oops; + pdata.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). */ - dummy_data->ecc_info.ecc_size = ramoops_ecc == 1 ? 16 : ramoops_ecc; + pdata.ecc_info.ecc_size = ramoops_ecc == 1 ? 16 : ramoops_ecc; dummy = platform_device_register_data(NULL, "ramoops", -1, - dummy_data, sizeof(struct ramoops_platform_data)); + &pdata, sizeof(pdata)); if (IS_ERR(dummy)) { pr_info("could not create platform device: %ld\n", PTR_ERR(dummy)); -- cgit v1.2.3-59-g8ed1b From 4c6c4d34536744f2c9e171ef5bb548a06a525501 Mon Sep 17 00:00:00 2001 From: Yue Hu Date: Thu, 31 Jan 2019 18:12:46 +0800 Subject: pstore: Avoid writing records with zero size Sometimes pstore_console_write() will write records with zero size to persistent ram zone, which is unnecessary. It will only increase resource consumption. Also adjust ramoops_write_kmsg_hdr() to have same logic if memory allocation fails. Signed-off-by: Yue Hu Signed-off-by: Kees Cook --- fs/pstore/platform.c | 3 +++ fs/pstore/ram.c | 6 ++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c index 2d1066ed3c28..75887a269b64 100644 --- a/fs/pstore/platform.c +++ b/fs/pstore/platform.c @@ -501,6 +501,9 @@ static void pstore_console_write(struct console *con, const char *s, unsigned c) { struct pstore_record record; + if (!c) + return; + pstore_record_init(&record, psinfo); record.type = PSTORE_TYPE_CONSOLE; diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index 1adb5e35c004..b2471d430048 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -352,8 +352,10 @@ static size_t ramoops_write_kmsg_hdr(struct persistent_ram_zone *prz, (time64_t)record->time.tv_sec, record->time.tv_nsec / 1000, record->compressed ? 'C' : 'D'); - WARN_ON_ONCE(!hdr); - len = hdr ? strlen(hdr) : 0; + if (WARN_ON_ONCE(!hdr)) + return 0; + + len = strlen(hdr); persistent_ram_write(prz, hdr, len); kfree(hdr); -- cgit v1.2.3-59-g8ed1b From 1e0f67a96aedd0b5fbd3ae743e68a26606ea98e8 Mon Sep 17 00:00:00 2001 From: Yue Hu Date: Wed, 23 Jan 2019 16:56:48 +0800 Subject: pstore/ram: Move initialization earlier Since only one single ramoops area allowed at a time, other probes (like device tree) are meaningless, as it will waste CPU resources. So let's check for being already initialized first. Signed-off-by: Yue Hu Signed-off-by: Kees Cook --- fs/pstore/ram.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index b2471d430048..d06e2f3e3bec 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -717,15 +717,6 @@ static int ramoops_probe(struct platform_device *pdev) phys_addr_t paddr; int err = -EINVAL; - if (dev_of_node(dev) && !pdata) { - pdata = &pdata_local; - memset(pdata, 0, sizeof(*pdata)); - - err = ramoops_parse_dt(pdev, pdata); - if (err < 0) - goto fail_out; - } - /* * Only a single ramoops area allowed at a time, so fail extra * probes. @@ -735,6 +726,15 @@ static int ramoops_probe(struct platform_device *pdev) goto fail_out; } + if (dev_of_node(dev) && !pdata) { + pdata = &pdata_local; + memset(pdata, 0, sizeof(*pdata)); + + err = ramoops_parse_dt(pdev, pdata); + if (err < 0) + goto fail_out; + } + /* Make sure we didn't get bogus platform data pointer. */ if (!pdata) { pr_err("NULL platform data\n"); -- cgit v1.2.3-59-g8ed1b From 47afd7ae65c737bfef67e9eb1424723d13b06787 Mon Sep 17 00:00:00 2001 From: Yue Hu Date: Fri, 1 Feb 2019 11:27:32 +0800 Subject: pstore/ram: Add kmsg hlen zero check to ramoops_pstore_write() If zero-length header happened in ramoops_write_kmsg_hdr(), that means we will not be able to read back dmesg record later, since it will be treated as invalid header in ramoops_pstore_read(). So we should not execute the following code but return the error. Signed-off-by: Yue Hu Signed-off-by: Kees Cook --- fs/pstore/ram.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index d06e2f3e3bec..ec0e1da0596a 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -425,6 +425,9 @@ static int notrace ramoops_pstore_write(struct pstore_record *record) /* Build header and append record contents. */ hlen = ramoops_write_kmsg_hdr(prz, record); + if (!hlen) + return -ENOMEM; + size = record->size; if (size + hlen > prz->buffer_size) size = prz->buffer_size - hlen; -- cgit v1.2.3-59-g8ed1b From 93ee4b7d9f0632690713aee604c49e298e634094 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Tue, 12 Feb 2019 13:09:36 -0800 Subject: pstore/ram: Avoid needless alloc during header write Since the header is a fixed small maximum size, just use a stack variable to avoid memory allocation in the write path. Signed-off-by: Kees Cook --- fs/pstore/ram.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index ec0e1da0596a..c5c685589e36 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -345,19 +345,15 @@ out: static size_t ramoops_write_kmsg_hdr(struct persistent_ram_zone *prz, struct pstore_record *record) { - char *hdr; + char hdr[36]; /* "===="(4), %lld(20), "."(1), %06lu(6), "-%c\n"(3) */ size_t len; - hdr = kasprintf(GFP_ATOMIC, RAMOOPS_KERNMSG_HDR "%lld.%06lu-%c\n", + len = scnprintf(hdr, sizeof(hdr), + RAMOOPS_KERNMSG_HDR "%lld.%06lu-%c\n", (time64_t)record->time.tv_sec, record->time.tv_nsec / 1000, record->compressed ? 'C' : 'D'); - if (WARN_ON_ONCE(!hdr)) - return 0; - - len = strlen(hdr); persistent_ram_write(prz, hdr, len); - kfree(hdr); return len; } -- cgit v1.2.3-59-g8ed1b