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

[Jingbai Ma <jingbai.ma@hp.com>]

On 09/17/2013 03:45 AM, Dave Anderson wrote:
> Recent testing on very large memory systems dictates that an
> update is required for the compressed kdump header generated
> by makedumpfile.
>
> The dumpfile header has this field, which was inherited from
> the old "diskdump" facility:
>
>   struct disk_dump_header {
>   ...
>          unsigned int            max_mapnr;      /* = max_mapnr */
>   ...
>
> and which, among other things, is used by the crash utility as a
> delimiter to determine whether a physical address read request is
> legitimate.  And obviously the field cannot handle PFN values greater
> than 32-bits.
>
> The makedumpfile source code does have its own max_mapnr representation
> in its DumpInfo structure in "makedumpfile.h":
>
>   struct DumpInfo {
>   ...
>          unsigned long long      max_mapnr;   /* number of page descriptor */
>   ...
>
> But in its "diskdump_mod.h" file, it carries forward the old diskdump
> header format, which has the 32-bit field:
>
>   struct disk_dump_header {
>   ...
>          unsigned int            max_mapnr;      /* = max_mapnr */
>   ...
>
> And here in "makedumpfile.c", the inadvertent truncation occurs
> when the PFN is greater than 32-bits:
>
>   int
>   write_kdump_header(void)
>   {
>   ...
>          dh->max_mapnr      = info->max_mapnr;
>   ...
>
> The 32-bit field has also been carried forward into the SADUMP header
> as well, which has this in "sadump_mod.h":
>
>   struct sadump_header {
> 	...
>          uint32_t max_mapnr;     /* = max_mapnr */	
> 	...
>
> 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@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];
};


-- 
Thanks,
Jingbai Ma

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec