On Thu, Sep 29, 2016 at 3:05 AM, Geliang Tang <geliangt...@gmail.com> wrote: > On Wed, Sep 28, 2016 at 12:45:13PM -0700, Kees Cook wrote: >> On Wed, Sep 28, 2016 at 3:32 AM, Geliang Tang <geliangt...@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.
Fair enough. :) I've adjusted the patch a bit with some additional pr_err()s added. It should appear in -next shortly. Thanks! -Kees > > Thanks. > > -Geliang > >> > >> > Signed-off-by: Geliang Tang <geliangt...@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 >> > -- Kees Cook Nexus Security