linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Michael Ellerman <mpe@ellerman.id.au>
To: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>, linuxppc-dev@ozlabs.org
Cc: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>,
	Mahesh Salgaonkar <mahesh@linux.ibm.com>
Subject: Re: [PATCH] powerpc/powernv/dump: Fix race while processing OPAL dump
Date: Fri, 09 Oct 2020 16:06:32 +1100	[thread overview]
Message-ID: <87a6ww0yzr.fsf@mpe.ellerman.id.au> (raw)
In-Reply-To: <20201007101742.40757-1-hegdevasant@linux.vnet.ibm.com>

Vasant Hegde <hegdevasant@linux.vnet.ibm.com> writes:
> diff --git a/arch/powerpc/platforms/powernv/opal-dump.c b/arch/powerpc/platforms/powernv/opal-dump.c
> index 543c816fa99e..7e6eeedec32b 100644
> --- a/arch/powerpc/platforms/powernv/opal-dump.c
> +++ b/arch/powerpc/platforms/powernv/opal-dump.c
> @@ -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) {
>  		kobject_put(&dump->kobj);
> -		return NULL;
> +		/* Drop reference count taken for bin file */
> +		kobject_put(&dump->kobj);
> +		return;
>  	}
>  
>  	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 reference count taken for bin file */
> +	kobject_put(&dump->kobj);
>  }

I think this would be better if it was reworked along the lines of:

aea948bb80b4 ("powerpc/powernv/elog: Fix race while processing OPAL error log event.")

cheers

  reply	other threads:[~2020-10-09  5:08 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-07 10:17 [PATCH] powerpc/powernv/dump: Fix race while processing OPAL dump Vasant Hegde
2020-10-09  5:06 ` Michael Ellerman [this message]
2020-10-17 12:26   ` Vasant Hegde

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87a6ww0yzr.fsf@mpe.ellerman.id.au \
    --to=mpe@ellerman.id.au \
    --cc=hegdevasant@linux.vnet.ibm.com \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=mahesh@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).