[PATCH v3] makedumpfile: fix a segmentation fault when physical address exceeds 8TB boundary

[Atsushi Kumagai]

>This patch intends to fix a segmentation fault when physical address exceeds
>8TB boundary.
>
>Changelog:
>v3:
>- Revise some comments according to Daisuke and Petr's feedback.
>
>v2:
>- Add more comments from Daisuke HATAYAMA.

Thanks!

Acked-by: Atsushi Kumagai <kumagai-atsushi at mxc.nes.nec.co.jp>

>In function is_on(), if the physical address higher than 8T, pfn (i) will
>greater than 2G, it will be a negative value and will cause a segmentation
>fault.
>is_on(char *bitmap, int i)
>{
>        return bitmap[i>>3] & (1 << (i & 7));
>}
>
>Daisuke's detailed analysis:
>static inline int
>is_dumpable(struct dump_bitmap *bitmap, unsigned long long pfn)
>{
>        off_t offset;
>        if (pfn == 0 || bitmap->no_block != pfn/PFN_BUFBITMAP) {
>                offset = bitmap->offset + BUFSIZE_BITMAP*(pfn/PFN_BUFBITMAP);
>                lseek(bitmap->fd, offset, SEEK_SET);
>                read(bitmap->fd, bitmap->buf, BUFSIZE_BITMAP);
>                if (pfn == 0)
>                        bitmap->no_block = 0;
>                else
>                        bitmap->no_block = pfn/PFN_BUFBITMAP;
>        }
>        return is_on(bitmap->buf, pfn%PFN_BUFBITMAP);
>}
>
>PFN_BUFBTIMAP is constant 32 K. So, it seems that the 4 byte byte
>length came here.
>
>But right shift to signed integer is implementation defined. We should
>not use right shift to signed integer. it looks gcc performs
>arithmetic shift and this bahaviour is buggy in case of is_on().
>
>static inline int
>is_dumpable_cyclic(char *bitmap, unsigned long long pfn, struct cycle *cycle)
>{
>        if (pfn < cycle->start_pfn || cycle->end_pfn <= pfn)
>                return FALSE;
>        else
>                return is_on(bitmap, pfn - cycle->start_pfn);
>}
>
>Simply, (pfn - cycle->start_pfn) could be ((info->max_mapnr - 1) - 0). It's
>possible to pass more than 2 Gi by using system with more than 8 TiB
>physical memory space.
>
>So, in function is_on()
>
>- i must be unsigned in order to make right shift operation
>  meaningful, and
>
>- i must have 8 byte for systems with more than 8 TiB physical memory
>  space.
>
>
>Petr's comments:
>Why an unsigned long for the variable i is not sufficient?
>It would be enough on 64-bit systems, where an unsigned long is just as big as
>an unsigned long long (both 64 bits). But on 32-bit systems, an unsigned long
>is 32-bit, but physical memory can be larger (e.g. 36 bits with PAE).
>
>
>Signed-off-by: Jingbai Ma <jingbai.ma at hp.com>
>---
> makedumpfile.h |    2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
>diff --git a/makedumpfile.h b/makedumpfile.h
>index 3d270c6..03d35a8 100644
>--- a/makedumpfile.h
>+++ b/makedumpfile.h
>@@ -1591,7 +1591,7 @@ int get_xen_info_ia64(void);
> #endif	/* s390x */
>
> static inline int
>-is_on(char *bitmap, int i)
>+is_on(char *bitmap, unsigned long long i)
> {
> 	return bitmap[i>>3] & (1 << (i & 7));
> }