From abce4d50fc28ff182bd7e0b714e21c0dfec62bd1 Mon Sep 17 00:00:00 2001

From: Jingbai Ma <jingbai.ma@hp.com>

Date: Fri, 18 Apr 2014 16:55:02 +0900

Subject: [PATCH] [PATCH v3] Fix a segmentation fault when physical address

 exceeds 8TB boundary.

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.

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@hp.com>

---

 makedumpfile.h | 2 +-

 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/makedumpfile.h b/makedumpfile.h

index a5826e0..33ab0cf 100644

--- a/makedumpfile-1.5.4/makedumpfile.h

+++ b/makedumpfile-1.5.4/makedumpfile.h

@@ -1546,7 +1546,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));

 }

-- 

1.9.3