Messages in this thread Patch in this message | | | Date | Thu, 29 Sep 2016 18:05:38 +0800 | From | Geliang Tang <> | Subject | Re: [PATCH] ramoops: add pdata NULL check to ramoops_probe |
| |
On Wed, Sep 28, 2016 at 12:45:13PM -0700, Kees Cook wrote: > On Wed, Sep 28, 2016 at 3:32 AM, Geliang Tang <geliangtang@gmail.com> wrote: > > When the pdata is NULL, ramoops_probe() segfaults. So this patch adds > > a NULL check to it. > > While I don't mind the check, is this even possible? A device > triggering a ramoops probe should already have a platform_data > (excepting the DT case which is already covered). Is there a situation > you can create to trigger this Oops? >
Hi Kees,
This did happen on my device recently.
We use platform_device_register() or platform_device_register_data() to set the ramoops_platform_data into dev->platform_data. If this ramoops_platform_data did not set successfully or if we pass a NULL parameter to platform_device_register(), we will get a NULL pdata in ramoops_probe(). This will trigger a kernel Oops.
Here is a test for this. If we set dummy_data to NULL in ramoops_register_dummy(), we will get the Oops:
@@ -747,6 +755,7 @@ static void ramoops_register_dummy(void) */ dummy_data->ecc_info.ecc_size = ramoops_ecc == 1 ? 16 : ramoops_ecc;
+ dummy_data = NULL; dummy = platform_device_register_data(NULL, "ramoops", -1, dummy_data, sizeof(struct ramoops_platform_data)); if (IS_ERR(dummy)) {
So I think this pdata NULL check is useful.
Thanks.
-Geliang
> > > > Signed-off-by: Geliang Tang <geliangtang@gmail.com> > > --- > > fs/pstore/ram.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c > > index 6ad831b..dd9832d 100644 > > --- a/fs/pstore/ram.c > > +++ b/fs/pstore/ram.c > > @@ -576,6 +576,9 @@ static int ramoops_probe(struct platform_device *pdev) > > if (cxt->max_dump_cnt) > > goto fail_out; > > > > + if (!pdata) > > + goto fail_out; > > + > > if (!pdata->mem_size || (!pdata->record_size && !pdata->console_size && > > !pdata->ftrace_size && !pdata->pmsg_size)) { > > pr_err("The memory size and the record/console size must be " > > -- > > 2.7.4 > >
| |