Re: [Crash-utility] [PATCH] fix read size in xendump.c

Isaku Yamahata wrote: > Hi. > Recently I twisted the xen xm dump-core format so that the section > order was changed for optimization. > Although it shouldn't affect analysis tools, it revealed a bug. > The crash utility tries to read the file beyond EOF. > So far it wasn't issue because of section ordering. > This patch fixes it by calculating read size correctly > The crash patch looks reasonable. But when you state that "So far it wasn't issue because of section ordering", it would seem that just the opposite would be true? Since the .xen_pages section has been moved closer to the beginning of the dumpfile, it would seem that the EOF read error would be *more likely* when using the older dumpfile format, where the .xen_pages section was located closer to the end of the dumpfile?

> Dave, Although I don't think that it causes serious issues, > Could you check whether the minor update of the format affects > the crash utility? > http://article.gmane.org/gmane.comp.emulators.xen.ia64/7531 > http://article.gmane.org/gmane.comp.emulators.xen.devel/42996 You'd have to make some sample dumpfiles available to me to verify the new format. I'm more concerned at the moment for backwards-compatibility, i.e., whether the crash patch will still work OK with the "older" xen ELF core format, which will be used in the RHEL5.1 update, and possibly beyond that. Thanks, Dave > > > --- crash-4.0-4.6-ORIG/xendump.c 2007-08-28 00:51:11.000000000 +0900 > +++ ./crash-4.0-4.6/xendump.c 2007-08-29 18:14:05.000000000 +0900 > @@ -1289,9 +1289,8 @@ xc_core_mfn_to_page(ulong mfn, char *pgb > > for (b = 0, idx = -1, done = FALSE; !done && (b < > nr_pages); b += MAX_BATCH_SIZE) { > - > - if (read(xd->xfd, tmp, sizeof(ulong) * > MAX_BATCH_SIZE) != - (MAX_BATCH_SIZE * sizeof(ulong))) { > + size_t size = sizeof(ulong) * MIN(MAX_BATCH_SIZE, nr_pages - b); > + if (read(xd->xfd, tmp, size) != size) { > error(INFO, "cannot read index page %d\n", b); > return NULL; > } > @@ -1348,13 +1347,11 @@ xc_core_elf_mfn_to_page(ulong mfn, char { > int i, b, idx, done; > off_t offset; > - size_t size; > uint nr_pages; > ulong tmp; > struct xen_dumpcore_p2m p2m_batch[MAX_BATCH_SIZE]; > > offset = xd->xc_core.header.xch_index_offset; > - size = sizeof(struct xen_dumpcore_p2m) * MAX_BATCH_SIZE; > nr_pages = xd->xc_core.header.xch_nr_pages; > > if (lseek(xd->xfd, offset, SEEK_SET) == -1) > @@ -1362,7 +1359,8 @@ xc_core_elf_mfn_to_page(ulong mfn, char > for (b = 0, idx = -1, done = FALSE; !done && (b < > nr_pages); b += MAX_BATCH_SIZE) { > - > + size_t size = sizeof(struct xen_dumpcore_p2m) * > + MIN(MAX_BATCH_SIZE, nr_pages - b); > if (read(xd->xfd, &p2m_batch[0], size) != size) { > error(INFO, "cannot read index page %d\n", b); > return NULL; > @@ -1437,9 +1435,9 @@ xc_core_mfn_to_page_index(ulong mfn) > nr_pages *= 2; > > for (b = 0; b < nr_pages; b += MAX_BATCH_SIZE) { > - > - if (read(xd->xfd, tmp, sizeof(ulong) * > MAX_BATCH_SIZE) != - (MAX_BATCH_SIZE * sizeof(ulong))) { > + size_t size = > + sizeof(ulong) * MIN(MAX_BATCH_SIZE, nr_pages - b); > + if (read(xd->xfd, tmp, size) != size) { > error(INFO, "cannot read index page %d\n", b); > return MFN_NOT_FOUND; > } > @@ -1469,20 +1467,19 @@ xc_core_elf_mfn_to_page_index(ulong mfn) > { > int i, b; > off_t offset; > - size_t size; > uint nr_pages; > ulong tmp; > struct xen_dumpcore_p2m p2m_batch[MAX_BATCH_SIZE]; > > offset = xd->xc_core.header.xch_index_offset; > - size = sizeof(struct xen_dumpcore_p2m) * MAX_BATCH_SIZE; > nr_pages = xd->xc_core.header.xch_nr_pages; > > if (lseek(xd->xfd, offset, SEEK_SET) == -1) > error(FATAL, "cannot lseek to page index\n"); > > for (b = 0; b < nr_pages; b += MAX_BATCH_SIZE) { > - > + size_t size = sizeof(struct xen_dumpcore_p2m) * > + MIN(MAX_BATCH_SIZE, nr_pages - b); > if (read(xd->xfd, &p2m_batch[0], size) != size) { > error(INFO, "cannot read index page %d\n", b); > return MFN_NOT_FOUND; > @@ -1563,8 +1560,9 @@ check_next_4: > nr_pages = xd->xc_core.header.xch_nr_pages; > > for (b = 0; b < nr_pages; b += MAX_BATCH_SIZE) { > - if (read(xd->xfd, tmp, sizeof(ulong) * > MAX_BATCH_SIZE) != > - (MAX_BATCH_SIZE * sizeof(ulong))) { > + size_t size = sizeof(ulong) * > + MIN(MAX_BATCH_SIZE, nr_pages - b); > + if (read(xd->xfd, tmp, size) != size) { > error(INFO, "cannot read index page > %d\n", b); > return FALSE; > } > @@ -1595,8 +1593,9 @@ show_64bit_mfns: > nr_pages = xd->xc_core.header.xch_nr_pages; > > for (b = 0; b < nr_pages; b += MAX_BATCH_SIZE) { > - if (read(xd->xfd, tmp64, sizeof(ulonglong) * > MAX_BATCH_SIZE) != > - (MAX_BATCH_SIZE * sizeof(ulonglong))) { > + size_t size = sizeof(ulonglong) * > + MIN(MAX_BATCH_SIZE, nr_pages - b); > + if (read(xd->xfd, tmp64, size) != size) { > error(INFO, "cannot read index page > %d\n", b); > return FALSE; > } > @@ -1709,13 +1708,11 @@ xc_core_elf_pfn_to_page_index(ulong pfn) > { > int i, b, start_index; > off_t offset; > - size_t size; > uint nr_pages; > ulong tmp; > struct xen_dumpcore_p2m p2m_batch[MAX_BATCH_SIZE]; > > offset = xd->xc_core.header.xch_index_offset; > - size = sizeof(struct xen_dumpcore_p2m) * MAX_BATCH_SIZE; > nr_pages = xd->xc_core.header.xch_nr_pages; > > /* > @@ -1744,7 +1741,8 @@ xc_core_elf_pfn_to_page_index(ulong pfn) > error(FATAL, "cannot lseek to page index\n"); > > for (b = start_index; b < nr_pages; b += MAX_BATCH_SIZE) { > - > + size_t size = sizeof(struct xen_dumpcore_p2m) * > + MIN(MAX_BATCH_SIZE, nr_pages - b); > if (read(xd->xfd, &p2m_batch[0], size) != size) { > error(INFO, "cannot read index page %d\n", b); > return PFN_NOT_FOUND; > @@ -1838,13 +1836,11 @@ xc_core_elf_pfn_valid(ulong pfn) > { > int i, b, start_index; > off_t offset; > - size_t size; > uint nr_pages; > ulong tmp; > uint64_t pfn_batch[MAX_BATCH_SIZE]; > > offset = xd->xc_core.header.xch_index_offset; > - size = sizeof(uint64_t) * MAX_BATCH_SIZE; > nr_pages = xd->xc_core.header.xch_nr_pages; > > /* > @@ -1873,7 +1869,8 @@ xc_core_elf_pfn_valid(ulong pfn) > error(FATAL, "cannot lseek to page index\n"); > > for (b = start_index; b < nr_pages; b += MAX_BATCH_SIZE) { > - > + size_t size = sizeof(uint64_t) * > + MIN(MAX_BATCH_SIZE, nr_pages - b); > if (read(xd->xfd, &pfn_batch[0], size) != size) { > error(INFO, "cannot read index page %d\n", b); > return PFN_NOT_FOUND; >