[PATCH] powerpc/powernv/dump: Fix race while processing OPAL dump

[Vasant Hegde]

On 10/9/20 10:36 AM, Michael Ellerman wrote:
> Vasant Hegde <hegdevasant at 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.")

Sure. Will fix it in v2.

-Vasant