[Skiboot] [PATCH v2 02/11] OPAL: nest feature detection

Madhavan Srinivasan maddy at linux.vnet.ibm.com
Thu Jul 30 13:28:01 AEST 2015



On Wednesday 29 July 2015 07:16 AM, Daniel Axtens wrote:
>> +int load_catalogue_lid(int loaded)
>> +{
>> +	if (loaded == OPAL_SUCCESS)
>> +		loaded = wait_for_resource_loaded(RESOURCE_ID_CATALOGUE,
>> +							RESOURCE_SUBID_NONE);
>> +
>> +	if (loaded != OPAL_SUCCESS) {
>> +		prerror("Nest_IMA: Error loading catalogue lid\n");
>> +		free(c_ptr->catalogue);
>> +		free(c_ptr);
>> +		return OPAL_RESOURCE;
>> +	}
> I don't see where preload_catalog_lid is called, but I assume the return
> code is passed to this. If preload_catalogue_lid() fails the first first
> allocation and returns OPAL_NO_MEM, you'll dereference a null pointer in
> the first free. If it fails the second allocation, the dereference will
> succeed, but then you'll attempt to free an invalid pointer in the first
> free.

Yes. Correct. This is possible. Will rewrite the check.


>
>> +
>> +	/*
>> +	 * Now that we have loaded the catalogue, check for the
>> +	 * catalog magic and make sure we have loaded what we wanted
>> +	 */
>> +	page0_ptr = (struct nest_catalog_page_0 *)CATALOGUE(c_ptr);
> You're taking a CATALOGUE, and casting it to a page0. I remember being
> confused about this in patch 1 as well. What's Page 0, and is it the
> same or different to catalog?

Catalogue is the meta data file which has information about
HW nest pmu events. Now, Catalogue has different structures
to carry this information, like event, group, formula and so on.
And the Calaogue first page (each catalogue page is 4k bytes)
"page 0" has information like magic#, version, build, and the
each structures locations in the file so on.  Since page0 and
catalogue are pointing to same start point, i took the catalogue ptr
and did type cast? And yes, I will remove the page0 ptr in the patch 1


Maddy
>> +	if (page0_ptr->magic != CATALOG_MAGIC) {
>> +		prerror("Nest_IMA: Error catalogue magic number mismatch\n");
>> +		free(c_ptr->catalogue);
>> +		free(c_ptr);
>> +		return OPAL_RESOURCE;
>> +	}
>> +



More information about the Skiboot mailing list