Re: [PATCH v2] powerpc/powernv: Restrict OPAL symbol map to only be readable by root

[Andrew Donnellan <ajd@linux.ibm.com>]

On 3/5/19 5:59 pm, Greg KH wrote:>> -static BIN_ATTR_RO(symbol_map, 0);
>> +static struct bin_attribute symbol_map_attr = {
>> +	.attr = {.name = "symbol_map", .mode = 0400},
>> +	.read = symbol_map_read
>> +};
> 
> There's no real need to rename the structure, right?  Why not just keep
> the bin_attr_symbol_map name?  That would make this patch even smaller.

No real need but it's locally more consistent with the rest of the PPC 
code. (Though perhaps the other cases should use the BIN_ATTR macro...)

Given this is for stable I'm happy to change that if the smaller patch 
is more acceptable.

> 
>>   static void opal_export_symmap(void)
>>   {
>> @@ -698,10 +701,10 @@ static void opal_export_symmap(void)
>>   		return;
>>   
>>   	/* Setup attributes */
>> -	bin_attr_symbol_map.private = __va(be64_to_cpu(syms[0]));
>> -	bin_attr_symbol_map.size = be64_to_cpu(syms[1]);
>> +	symbol_map_attr.private = __va(be64_to_cpu(syms[0]));
>> +	symbol_map_attr.size = be64_to_cpu(syms[1]);
>>   
>> -	rc = sysfs_create_bin_file(opal_kobj, &bin_attr_symbol_map);
>> +	rc = sysfs_create_bin_file(opal_kobj, &symbol_map_attr);
> 
> Meta-comment, odds are you are racing userspace when you create this
> sysfs file, why not add it to the device's default attributes so the
> driver core creates it for you at the correct time?

I was not previously aware of default attributes...

Are we actually racing against userspace in a subsys initcall?

-- 
Andrew Donnellan              OzLabs, ADL Canberra
ajd@linux.ibm.com             IBM Australia Limited