lkml.org 
[lkml]   [2016]   [Oct]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.8 025/140] pstore/ramoops: fixup driver removal
    Date
    4.8-stable review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

    commit 4407de74df18ed405cc5998990004c813ccfdbde upstream.

    A basic rmmod ramoops segfaults. Let's see why.

    Since commit 34f0ec82e0a9 ("pstore: Correct the max_dump_cnt clearing of
    ramoops") sets ->max_dump_cnt to zero before looping over ->przs but we
    didn't use it before that either.

    And since commit ee1d267423a1 ("pstore: add pstore unregister") we free
    that memory on rmmod.

    But even then, we looped until a NULL pointer or ERR. I don't see where
    it is ensured that the last member is NULL. Let's try this instead:
    simply error recovery and free. Clean up in error case where resources
    were allocated. And then, in the free path, rely on ->max_dump_cnt in
    the free path.

    Cc: Anton Vorontsov <anton@enomsg.org>
    Cc: Colin Cross <ccross@android.com>
    Cc: Kees Cook <keescook@chromium.org>
    Cc: Tony Luck <tony.luck@intel.com>
    Cc: Namhyung Kim <namhyung@kernel.org>
    Acked-by: Namhyung Kim <namhyung@kernel.org>
    Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
    Signed-off-by: Kees Cook <keescook@chromium.org>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    fs/pstore/ram.c | 17 ++++++++++++-----
    1 file changed, 12 insertions(+), 5 deletions(-)

    --- a/fs/pstore/ram.c
    +++ b/fs/pstore/ram.c
    @@ -377,13 +377,14 @@ static void ramoops_free_przs(struct ram
    {
    int i;

    - cxt->max_dump_cnt = 0;
    if (!cxt->przs)
    return;

    - for (i = 0; !IS_ERR_OR_NULL(cxt->przs[i]); i++)
    + for (i = 0; i < cxt->max_dump_cnt; i++)
    persistent_ram_free(cxt->przs[i]);
    +
    kfree(cxt->przs);
    + cxt->max_dump_cnt = 0;
    }

    static int ramoops_init_przs(struct device *dev, struct ramoops_context *cxt,
    @@ -408,7 +409,7 @@ static int ramoops_init_przs(struct devi
    GFP_KERNEL);
    if (!cxt->przs) {
    dev_err(dev, "failed to initialize a prz array for dumps\n");
    - goto fail_prz;
    + goto fail_mem;
    }

    for (i = 0; i < cxt->max_dump_cnt; i++) {
    @@ -419,6 +420,11 @@ static int ramoops_init_przs(struct devi
    err = PTR_ERR(cxt->przs[i]);
    dev_err(dev, "failed to request mem region (0x%zx@0x%llx): %d\n",
    cxt->record_size, (unsigned long long)*paddr, err);
    +
    + while (i > 0) {
    + i--;
    + persistent_ram_free(cxt->przs[i]);
    + }
    goto fail_prz;
    }
    *paddr += cxt->record_size;
    @@ -426,7 +432,9 @@ static int ramoops_init_przs(struct devi

    return 0;
    fail_prz:
    - ramoops_free_przs(cxt);
    + kfree(cxt->przs);
    +fail_mem:
    + cxt->max_dump_cnt = 0;
    return err;
    }

    @@ -659,7 +667,6 @@ static int ramoops_remove(struct platfor
    struct ramoops_context *cxt = &oops_cxt;

    pstore_unregister(&cxt->pstore);
    - cxt->max_dump_cnt = 0;

    kfree(cxt->pstore.buf);
    cxt->pstore.bufsize = 0;

    \
     
     \ /
      Last update: 2016-10-26 15:33    [W:4.497 / U:0.788 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site