Messages in this thread | | | From | Kees Cook <> | Date | Tue, 22 Jan 2019 16:32:11 +1300 | Subject | Re: [PATCH] pstore/ram: Replace dummy_data heap memory with stack memory |
| |
On Tue, Jan 22, 2019 at 2:37 PM Yue Hu <huyue2@yulong.com> wrote: > > From e37cbd4d22eae55c034536817b22d429ba0ae27a Mon Sep 17 00:00:00 2001
Can you check your MUA? It turns out this email was damaged in a few ways: - something broke enough that it never appeared on patchwork (maybe not on lkml either?) - lines are wrapped in the body
> From: Yue Hu <huyue2@yulong.com> > Date: Mon, 21 Jan 2019 18:20:41 +0800 > Subject: [PATCH] pstore/ram: Replace dummy_data heap memory with stack > memory
like here
> > 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 <huyue2@yulong.com> > --- > fs/pstore/ram.c | 34 +++++++++++++--------------------- > 1 file changed, 13 insertions(+), 21 deletions(-) > > diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c > index 96f7d32..8db1f7f 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) > { > @@ -896,13 +895,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 > @@ -913,30 +911,24 @@ static void __init ramoops_register_dummy(void) > > pr_info("using module parameters\n"); > > - dummy_data = kzalloc(sizeof(*dummy_data), GFP_KERNEL);
The zeroing here is required.
> - 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;
I added a memset() to zero here to compensate.
> + 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(struct ramoops_platform_data));
I made this more robust by using sizeof(pdata) instead of an explicit struct.
> if (IS_ERR(dummy)) { > pr_info("could not create platform device: %ld\n", > PTR_ERR(dummy)); > -- > 1.9.1
The result should be visible in linux-next soon.
-- Kees Cook
| |