[BUG] [compressed kdump / SADUMP] makedumpfile header truncation error

[HATAYAMA Daisuke]

(2013/09/17 16:12), Jingbai Ma wrote:
> On 09/17/2013 02:55 PM, HATAYAMA Daisuke wrote:
>> (2013/09/17 13:36), Jingbai Ma wrote:
>> <cut>
>>>>
>>>> And when these header structures change, the crash utility will need
>>>> to be changed accordingly.
>>>>
>>>> Preferably for backwards-compatibility, a new header_version can be
>>>> created, with the new expanded field located in the kdump_sub_header
>>>> so that the original base structure can remain as-is. But I leave that
>>>> up to the maintainers.
>>>>
>>>> Thanks,
>>>> Dave
>>>>
>>>> _______________________________________________
>>>> kexec mailing list
>>>> kexec at lists.infradead.org
>>>> http://lists.infradead.org/mailman/listinfo/kexec
>>>>
>>>
>>> For the persistent data structures, we should use more precision
>>> declaration int32_t, int64_t, uint64_t instead of ambiguous int, long
>>> int, long long int.
>>> For example, we can change structure disk_dump_header as below:
>>> struct disk_dump_header {
>>> char signature[SIG_LEN]; /* = "KDUMP " */
>>> int32_t header_version; /* Dump header version */
>>> struct new_utsname utsname; /* copy of system_utsname */
>>> struct timeval timestamp; /* Time stamp */
>>> uint32_t status; /* Above flags */
>>> int32_t block_size; /* Size of a block in byte */
>>> int32_t sub_hdr_size; /* Size of arch dependent
>>> header in blocks */
>>> uint32_t bitmap_blocks; /* Size of Memory bitmap in
>>> block */
>>> uint64_t max_mapnr; /* = max_mapnr */
>>> uint32_t total_ram_blocks;/* Number of blocks should be
>>> written */
>>> uint32_t device_blocks; /* Number of total blocks in
>>> * the dump device */
>>> uint32_t written_blocks; /* Number of written blocks */
>>> uint32_t current_cpu; /* CPU# which handles dump */
>>> int32_t nr_cpus; /* Number of CPUs */
>>> struct task_struct *tasks[0];
>>> };
>>>
>>>
>>
>> Looking at arch directory, this structure is used on x86, x86_64, ppc,
>> ppc64, s390
>> and ia64. Does this definition work well on all the architectures?
>>
>> tasks member has obviously different length in each architecture but
>> this member
>> is never used now.
>>
>> More worse is kdump_sub_header structure. Obviously, unsigned long has
>> different
>> length on x86 and x86_64, though you have already noticed this. I don't
>> know ABI on
>> other architectures. Sorry.
>>
>> /*
>> * Sub header for KDUMP
>> * But Common header of KDUMP is disk_dump_header of diskdump.
>> */
>> struct kdump_sub_header {
>> unsigned long phys_base;
>> int dump_level; /* header_version 1 and later */
>> int split; /* header_version 2 and later */
>> unsigned long start_pfn; /* header_version 2 and later */
>> unsigned long end_pfn; /* header_version 2 and later */
>> off_t offset_vmcoreinfo;/* header_version 3 and later */
>> unsigned long size_vmcoreinfo; /* header_version 3 and later */
>> off_t offset_note; /* header_version 4 and later */
>> unsigned long size_note; /* header_version 4 and later */
>> off_t offset_eraseinfo; /* header_version 5 and later */
>> unsigned long size_eraseinfo; /* header_version 5 and later */
>> };
>>
>
> int32_t, int64_t, uint64_t, etc ... are parts of C99 standard:
> http://en.wikipedia.org/wiki/C_data_types
> All there types have been supported by GCC, so them should work on all the architectures.
>
> Although change these persistent data structure will affect both makedumpfile and crash utility, but we will benefit from the consistent data structures independent from architectures. We can analyze a dumpfile on a OS with different architecture than the crashed OS.
>
>

I know stdint.h things and usefulness if we can use crash and makedumpfile
for a multiple architectures on single arch. In fact, crash already supports
cross platform build among some architectures thanks to Dave.

My question came from the fact that it looks like you introduced a single
modified kdump_sub_header structure for all the architectures. They might
have different combination of length between int and long and maybe
also have other each architecture specific incompatibility. It wouldn't
work well.

But from your reply, I think you mean a fully new header for kdump-compressed
format, right? If so, it must work well. But of course you need to modify
both of makedumpfile and crash utility to support it.

-- 
Thanks.
HATAYAMA, Daisuke