lkml.org 
[lkml]   [2020]   [Oct]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.8 518/633] powerpc/powernv/dump: Fix race while processing OPAL dump
    Date
    From: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>

    [ Upstream commit 0a43ae3e2beb77e3481d812834d33abe270768ab ]

    Every dump reported by OPAL is exported to userspace through a sysfs
    interface and notified using kobject_uevent(). The userspace daemon
    (opal_errd) then reads the dump and acknowledges that the dump is
    saved safely to disk. Once acknowledged the kernel removes the
    respective sysfs file entry causing respective resources to be
    released including kobject.

    However it's possible the userspace daemon may already be scanning
    dump entries when a new sysfs dump entry is created by the kernel.
    User daemon may read this new entry and ack it even before kernel can
    notify userspace about it through kobject_uevent() call. If that
    happens then we have a potential race between
    dump_ack_store->kobject_put() and kobject_uevent which can lead to
    use-after-free of a kernfs object resulting in a kernel crash.

    This patch fixes this race by protecting the sysfs file
    creation/notification by holding a reference count on kobject until we
    safely send kobject_uevent().

    The function create_dump_obj() returns the dump object which if used
    by caller function will end up in use-after-free problem again.
    However, the return value of create_dump_obj() function isn't being
    used today and there is no need as well. Hence change it to return
    void to make this fix complete.

    Fixes: c7e64b9ce04a ("powerpc/powernv Platform dump interface")
    Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
    Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
    Link: https://lore.kernel.org/r/20201017164210.264619-1-hegdevasant@linux.vnet.ibm.com
    Signed-off-by: Sasha Levin <sashal@kernel.org>
    ---
    arch/powerpc/platforms/powernv/opal-dump.c | 41 +++++++++++++++-------
    1 file changed, 29 insertions(+), 12 deletions(-)

    diff --git a/arch/powerpc/platforms/powernv/opal-dump.c b/arch/powerpc/platforms/powernv/opal-dump.c
    index 543c816fa99ef..0e6693bacb7e7 100644
    --- a/arch/powerpc/platforms/powernv/opal-dump.c
    +++ b/arch/powerpc/platforms/powernv/opal-dump.c
    @@ -318,15 +318,14 @@ static ssize_t dump_attr_read(struct file *filep, struct kobject *kobj,
    return count;
    }

    -static struct dump_obj *create_dump_obj(uint32_t id, size_t size,
    - uint32_t type)
    +static void create_dump_obj(uint32_t id, size_t size, uint32_t type)
    {
    struct dump_obj *dump;
    int rc;

    dump = kzalloc(sizeof(*dump), GFP_KERNEL);
    if (!dump)
    - return NULL;
    + return;

    dump->kobj.kset = dump_kset;

    @@ -346,21 +345,39 @@ static struct dump_obj *create_dump_obj(uint32_t id, size_t size,
    rc = kobject_add(&dump->kobj, NULL, "0x%x-0x%x", type, id);
    if (rc) {
    kobject_put(&dump->kobj);
    - return NULL;
    + return;
    }

    + /*
    + * As soon as the sysfs file for this dump is created/activated there is
    + * a chance the opal_errd daemon (or any userspace) might read and
    + * acknowledge the dump before kobject_uevent() is called. If that
    + * happens then there is a potential race between
    + * dump_ack_store->kobject_put() and kobject_uevent() which leads to a
    + * use-after-free of a kernfs object resulting in a kernel crash.
    + *
    + * To avoid that, we need to take a reference on behalf of the bin file,
    + * so that our reference remains valid while we call kobject_uevent().
    + * We then drop our reference before exiting the function, leaving the
    + * bin file to drop the last reference (if it hasn't already).
    + */
    +
    + /* Take a reference for the bin file */
    + kobject_get(&dump->kobj);
    rc = sysfs_create_bin_file(&dump->kobj, &dump->dump_attr);
    - if (rc) {
    + if (rc == 0) {
    + kobject_uevent(&dump->kobj, KOBJ_ADD);
    +
    + pr_info("%s: New platform dump. ID = 0x%x Size %u\n",
    + __func__, dump->id, dump->size);
    + } else {
    + /* Drop reference count taken for bin file */
    kobject_put(&dump->kobj);
    - return NULL;
    }

    - pr_info("%s: New platform dump. ID = 0x%x Size %u\n",
    - __func__, dump->id, dump->size);
    -
    - kobject_uevent(&dump->kobj, KOBJ_ADD);
    -
    - return dump;
    + /* Drop our reference */
    + kobject_put(&dump->kobj);
    + return;
    }

    static irqreturn_t process_dump(int irq, void *data)
    --
    2.25.1


    \
     
     \ /
      Last update: 2020-10-27 16:40    [W:2.983 / U:0.584 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site