Re: [PATCH v3] vmcore: prevent PT_NOTE p_memsz overflow duringheader update

[Pearson, Greg]

On 02/05/2014 06:39 AM, Vivek Goyal wrote:
> On Tue, Feb 04, 2014 at 04:25:52PM -0700, Greg Pearson wrote:
>
> [..]
>> diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c
>> index 2ca7ba0..88d4585 100644
>> --- a/fs/proc/vmcore.c
>> +++ b/fs/proc/vmcore.c
>> @@ -468,17 +468,24 @@ static int __init update_note_header_size_elf64(const Elf64_Ehdr *ehdr_ptr)
>>   			return rc;
>>   		}
>>   		nhdr_ptr = notes_section;
>> -		while (real_sz < max_sz) {
>> -			if (nhdr_ptr->n_namesz == 0)
>> -				break;
>> +		while (nhdr_ptr->n_namesz != 0) {
>>   			sz = sizeof(Elf64_Nhdr) +
>>   				((nhdr_ptr->n_namesz + 3) & ~3) +
>>   				((nhdr_ptr->n_descsz + 3) & ~3);
>> +			if ((real_sz + sz) > max_sz) {
>> +				pr_warn("Warning: Exceeded p_memsz, dropping PT_NOTE entry n_namesz=0x%x, n_descsz=0x%x\n",
>> +					nhdr_ptr->n_namesz, nhdr_ptr->n_descsz);
> You will need line break in pr_warn(). Too long a line. Limit it 80
> columns per line.

The checkpatch.pl script issues a warning when you break quoted strings, 
I have no personal preference. Just let me know if you want me to ignore 
the checkpatch warning and conform to the 80 column per line limit.

Also, I'm still debugging the reason why the corrupt entries show up, 
I'll continue to work on that.

Thanks

--
Greg


>
>> +				break;
>> +			}
>>   			real_sz += sz;
>>   			nhdr_ptr = (Elf64_Nhdr*)((char*)nhdr_ptr + sz);
>>   		}
>>   		kfree(notes_section);
>>   		phdr_ptr->p_memsz = real_sz;
>> +		if (real_sz == 0) {
>> +			pr_warn("Warning: Zero PT_NOTE entries found\n");
>> +			return -EINVAL;
> Given the fact that this is the first time I have heard about a PT_NOTE
> being corrup, I will be fine with this patch too. If one encounters
> an empty PT_NOTE, error out and don't create vmcore.
>
> So please repost this patch with line lenght fixed.
>
> Thanks
> Vivek
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/