[Crash-utility] [crash-utility/crash] Bitmap calculation overflow in large physical address scenario

[Dave Anderson]

----- Original Message -----
> 
> 
> Hi ，
> 
> I used crash to analyze a vmcore and found that when crash calculates the
> bitmap and pfn, an integer overflow occurs, causing the crash parsing to
> fail.

Hi Jialong,

I verified the patch for backwards compatibility, and after getting an ACK from 
makedumpfile maintainer Kazuhito Hagio, which also requires a patch, I have queued
this patch for crash-7.2.9:

  https://github.com/crash-utility/crash/commit/b80b16549e24769c7d5fe3a10c4b1a1c4b5161f3

Thanks,
  Dave


> 
> 
> 
> [root at localhost userspace]# crash vmlinux vmcore
> 
> ……
> 
> reason: variable overflow causes a logic error in crash.
> 
> crash: page excluded: kernel virtual address: ffff0000089c9100 type:
> "kernel_config_data"
> 
> WARNING: cannot read kernel_config_data
> 
> crash: page excluded: kernel virtual address: ffff00000911b938 type:
> "possible"
> 
> WARNING: cannot read cpu_possible_map
> 
> crash: page excluded: kernel virtual address: ffff00000911b8b8 type:
> "present"
> 
> WARNING: cannot read cpu_present_map
> 
> crash: page excluded: kernel virtual address: ffff00000911b838 type: "online"
> 
> WARNING: cannot read cpu_online_map
> 
> crash: page excluded: kernel virtual address: ffff00000911b9b8 type: "active"
> 
> WARNING: cannot read cpu_active_map
> 
> crash: page excluded: kernel virtual address: ffff0000093ec9d0 type:
> "shadow_timekeeper xtime_sec"
> 
> crash: page excluded: kernel virtual address: ffff000009124d2c type:
> "init_uts_ns"
> 
> crash: vmlinux and vmcore do not match!
> 
> 
> 
> And the /proc/iomem info:
> 
> 2e69267000-2fffffffff : System RAM
> 
> ......
> 
> 602770ecf000-6027ffffffff : System RAM
> 
> 
> 
> Here is the process of my analysis ：
> 
> 1. calculate bitmap_len overflow in function read_dump_header()
> 
> int block_size=(int)sysconf(_SC_PAGESIZE);
> 
> off_t bitmap_len;
> 
> unsigned int bitmap_blocks;
> 
> ...
> 
> bitmap_len = block_size * header->bitmap_blocks;
> 
> 
> 
> here block_size = 4096, header->bitmap_blocks = 0x180a00, so bitmap_len =
> 0x180a00000
> 
> but block_size is integer type, bitmap_blocks is unsigned int type, so
> block_size * header->bitmap_blocks > MAX(unsigned int)
> 
> bitmap_len overflow.
> 
> 
> 
> 2. calculate data_offset overflow in function read_dump_header()
> 
> dd->data_offset = (1 + header->sub_hdr_size + header->bitmap_blocks) *
> header->block_size;
> 
> 
> 
> info in makedumpfile:
> 
> info->offset_bitmap1=0x15000 info->len_bitmap=0x182000000
> bit2_offset=0xc1015000
> 
> in read_dump_header():
> 
> info->len_bitmap = header->bitmap_blocks * header->block_size = 0x182000000
> is greater than the range represented by integers.
> 
> The members of the following expression are all int type, but the calculation
> result exceeds MAX(int),
> 
> dd->data_offset = (1 + header->sub_hdr_size + header->bitmap_blocks) *
> header->block_size = 0x82015000,
> 
> The correct value is 0x182015000.
> 
> 
> 
> 3. byte parameter overflow in function get_bit()
> 
> static inline int get_bit(char *map, int byte, int bit)
> 
> {
> 
> return map[byte] & (1<<bit);
> 
> }
> 
> static inline int page_is_ram(unsigned long nr)
> 
> {
> 
> return get_bit(dd->bitmap, nr >> 3, nr & 7);
> 
> }
> 
> 
> 
> if nr=0x6027fff4f, 0x6027fff4f >> 3 => 0xC04FFFE9 > MAX(int),
> 
> so byte parameter overflow when call get_bit.
> 
> 
> 
> The following is my patch, please review. Thanks.
> 
> Signed-off-by: Jialong Chen <chenjialong at huawei.com>
> 
> ---
> 
> diskdump.c | 6 +++---
> 
> 1 file changed, 3 insertions(+), 3 deletions(-)
> 
> 
> 
> diff --git a/diskdump.c b/diskdump.c
> 
> index e88243e..328c932 100644
> 
> --- a/diskdump.c
> 
> +++ b/diskdump.c
> 
> @@ -233,7 +233,7 @@ clean_diskdump_data(void)
> 
> }
> 
> 
> 
> static inline int
> 
> -get_bit(char *map, int byte, int bit)
> 
> +get_bit(char *map, unsigned long byte, int bit)
> 
> {
> 
> return map[byte] & (1<<bit);
> 
> }
> 
> @@ -694,7 +694,7 @@ restart:
> 
> dd->max_mapnr = header->max_mapnr;
> 
> 
> 
> /* read memory bitmap */
> 
> - bitmap_len = block_size * header->bitmap_blocks;
> 
> + bitmap_len = (off_t)block_size * header->bitmap_blocks;
> 
> dd->bitmap_len = bitmap_len;
> 
> 
> 
> offset = (off_t)block_size * (1 + header->sub_hdr_size);
> 
> @@ -744,7 +744,7 @@ restart:
> 
> memcpy(dd->dumpable_bitmap, dd->bitmap, bitmap_len);
> 
> 
> 
> dd->data_offset
> 
> - = (1 + header->sub_hdr_size + header->bitmap_blocks)
> 
> + = (1UL + header->sub_hdr_size + header->bitmap_blocks)
> 
> * header->block_size;
> 
> 
> 
> dd->header = header;
> 
> --
> 
> 1.7.12.4
> 
> 
> 
> --
> Crash-utility mailing list
> Crash-utility at redhat.com
> https://www.redhat.com/mailman/listinfo/crash-utility