(2014/04/17 16:59), Jingbai Ma wrote: > This patch intends to fix a segmentation fault when physical address exceeds > 8TB boundary. > > Changelog: > 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 - 0). It's More precisely, here should have been ((info->max_mapnr - 1) - 0) since info->max_mapnr doesn't belong to target physical memory space... > 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. > > > 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)); > } > -- Thanks. HATAYAMA, Daisuke