[Skiboot] [RESEND PATCH 3/8] nvram_format: Fix unterminated string warning

Kamalesh Babulal kamalesh at linux.vnet.ibm.com
Thu Jun 25 15:01:49 AEST 2015



On 06/24/2015 01:20 PM, Stewart Smith wrote:
> Kamalesh Babulal <kamalesh at linux.vnet.ibm.com> writes:
>> Sorry previous patch had changes related to cross compiler.
>>
>> --8<--
>> We write NVRAM_NAME_FREE, which is of strlen(12) into
>> struct chrp_nvram_hdr->name[12] using strncpy. This could
>> result in an unterminated string.
>>
>> This patch alters the length of NVRAM_NAME_FREE length 11
>> and as nvram_image is already memset to zero. Note that the
>> NVRAM_NAME_FREE is local to this file.
>>
>> Fixes Coverity defect #97817.
>>
>> Signed-off-by: Kamalesh Babulal <kamalesh at linux.vnet.ibm.com>
>> ---
>>  core/nvram.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/core/nvram.c b/core/nvram.c
>> index f25d6aa..f59fb82 100644
>> --- a/core/nvram.c
>> +++ b/core/nvram.c
>> @@ -63,7 +63,7 @@ struct chrp_nvram_hdr {
>>  
>>  #define NVRAM_NAME_COMMON	"common"
>>  #define NVRAM_NAME_FW_PRIV	"ibm,skiboot"
>> -#define NVRAM_NAME_FREE		"wwwwwwwwwwww"
>> +#define NVRAM_NAME_FREE		"wwwwwwwwwww"
>>  
>>  /* 64k should be enough, famous last words... */
>>  #define NVRAM_SIZE_COMMON	0x10000
>> @@ -117,7 +117,7 @@ static void nvram_format(void)
>>  	h = nvram_image + offset;
>>  	h->sig = NVRAM_SIG_FREE;
>>  	h->len = (nvram_size - offset) >> 4;
>> -	strncpy(h->name, NVRAM_NAME_FREE, 12);
>> +	strncpy(h->name, NVRAM_NAME_FREE, 11);
>>  	h->cksum = chrp_nv_cksum(h);
>>  
>>  	/* Write the whole thing back */
> From PAPR section 8.4: "The name field is a 12 byte string (or a
> NULL-terminated string of less than 12 bytes) used to identify a
> particular NVRAM partition within a signature group"
>
> and from PAPR section 8.4.2:
> "R1–8.4.2–2. All Free Space NVRAM partitions must have the name field
> set to 0x7...77."
>
> Which reads to me like we do actually need to have the 12 characters
> there.
>
> So, instead, I've merged one that just uses memcpy instead, and adds a
> bunch of unit tests for the NVRAM format.

Thanks for review. Will take a look at spec before altering the length
of structures,

-- 
Cheers,
Kamalesh.



More information about the Skiboot mailing list