On Wed, Sep 28, 2016 at 12:45:13PM -0700, Kees Cook wrote:
> On Wed, Sep 28, 2016 at 3:32 AM, Geliang Tang <[email protected]> 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 <[email protected]>
> > ---
> > 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
> >
