From: Jingbai Ma <jingbai.ma@xxxxxx>
Subject: [PATCH] makedumpfile: Fix a segment fault in dumping small ELF segment
``small ELF segment'' is wrong. This issue occurs if the starting or
ending address of the ELF segment is not aligned to multiple of 8
pages. Could you correct the subject?
Date: Fri, 28 Mar 2014 20:26:34 +0800
> This patch fixs a bug if the size of an ELF segment less than 8 pages.
>
Could you show me /proc/iomem and an output of readelf -l of the ELF
vmcore? I'm interested in the segment.
> In function create_1st_bitmap_cyclic() and initialize_2nd_bitmap_cyclic(),
> there are the same code:
>
> pfn_start_roundup = roundup(pfn_start, BITPERBYTE);
> pfn_end_round = round(pfn_end, BITPERBYTE);
>
> for (pfn = pfn_start; pfn < pfn_start_roundup; pfn++) {
> if (set_bit_on_1st_bitmap(pfn))
> pfn_bitmap1++;
> }
>
> In case:
> pfn_start=0xe762c, pfn_start_roundup=0xe7630
> pfn_end=0xe762d, pfn_end_round=0xe7628
> This code will set incorrect bits in the bitmap.
> In function readpage_elf():
>
> if (!offset1) {
> phys_start = page_head_to_phys_start(paddr);
> offset1 = paddr_to_offset(phys_start);
> frac_head = phys_start - paddr;
> memset(bufptr, 0, frac_head);
> }
>
> The invalid paddr couldn't be found, so phys_start will be zero, and frac_head
> will be negative, then memset will cause a segment fault.
>
> Signed-off-by: Jingbai Ma <jingbai.ma at hp.com>
> ---
> makedumpfile.c | 26 +++++++++++++++-----------
> 1 files changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/makedumpfile.c b/makedumpfile.c
> index ef08d91..21330b7 100644
> --- a/makedumpfile.c
> +++ b/makedumpfile.c
> @@ -4424,8 +4424,9 @@ create_1st_bitmap_cyclic()
> if (pfn_start >= pfn_end)
> continue;
>
> - pfn_start_roundup = roundup(pfn_start, BITPERBYTE);
> - pfn_end_round = round(pfn_end, BITPERBYTE);
> + pfn_start_roundup = MIN(roundup(pfn_start, BITPERBYTE),
> + pfn_end);
Please add two more tabs in the line of the second argument of MIN()
like this for readability:
+ pfn_start_roundup = MIN(roundup(pfn_start, BITPERBYTE),
+ pfn_end);
> + pfn_end_round = MAX(round(pfn_end, BITPERBYTE), pfn_start);
>
> for (pfn = pfn_start; pfn < pfn_start_roundup; pfn++) {
> if (set_bit_on_1st_bitmap(pfn))
> @@ -4443,10 +4444,11 @@ create_1st_bitmap_cyclic()
> pfn_bitmap1 += (pfn_end_byte - pfn_start_byte) * BITPERBYTE;
> }
>
> - for (pfn = pfn_end_round; pfn < pfn_end; pfn++) {
> - if (set_bit_on_1st_bitmap(pfn))
> - pfn_bitmap1++;
> - }
> + if (pfn_end_round > pfn_start)
> + for (pfn = pfn_end_round; pfn < pfn_end; pfn++) {
> + if (set_bit_on_1st_bitmap(pfn))
> + pfn_bitmap1++;
> + }
Please add { ... } for the outer if to encolose the for statement like this:
+ if (pfn_end_round > pfn_start) {
+ for (pfn = pfn_end_round; pfn < pfn_end; pfn++) {
+ if (set_bit_on_1st_bitmap(pfn))
+ pfn_bitmap1++;
+ }
+ }
> }
> pfn_memhole -= pfn_bitmap1;
>
> @@ -4532,8 +4534,9 @@ initialize_2nd_bitmap_cyclic(void)
> if (pfn_start >= pfn_end)
> continue;
>
> - pfn_start_roundup = roundup(pfn_start, BITPERBYTE);
> - pfn_end_round = round(pfn_end, BITPERBYTE);
> + pfn_start_roundup = MIN(roundup(pfn_start, BITPERBYTE),
> + pfn_end);
Similr.
> + pfn_end_round = MAX(round(pfn_end, BITPERBYTE), pfn_start);
>
> for (pfn = pfn_start; pfn < pfn_start_roundup; ++pfn)
> if (!set_bit_on_2nd_bitmap_for_kernel(pfn))
> @@ -4548,9 +4551,10 @@ initialize_2nd_bitmap_cyclic(void)
> pfn_end_byte - pfn_start_byte);
> }
>
> - for (pfn = pfn_end_round; pfn < pfn_end; ++pfn)
> - if (!set_bit_on_2nd_bitmap_for_kernel(pfn))
> - return FALSE;
> + if (pfn_end_round > pfn_start)
> + for (pfn = pfn_end_round; pfn < pfn_end; ++pfn)
> + if (!set_bit_on_2nd_bitmap_for_kernel(pfn))
> + return FALSE;
Similar.
> }
>
> return TRUE;
>
==
Thanks.
HATAYAMA, Daisuke
