<!doctype html public "-//w3c//dtd html 4.0 transitional//en"> <html> <tt>Dave Anderson wrote:</tt> <blockquote TYPE=CITE><tt>Itsuro ODA wrote:</tt><tt></tt> <tt>> Hi Dave,</tt> <tt>></tt> <tt>> > - Introduced support for xendumps of para-virtualized ia64 kernels.</tt> <tt>> > It should be noted that currently the ia64 Xen kernel does not</tt> <tt>> > lay down a switch_stack for the panic task, so only raw "bt -t"</tt> <tt>> > backtraces can be done on the panic task. (anderson@redhat.com)</tt> <tt>></tt> <tt>> page_index may contain INVALID_MFN (0xffffffffffffffffUL).</tt> <tt>> I think it is better to check it.</tt> <tt>> Here is a patch for this.</tt> <tt>></tt> <tt>> ---</tt> <tt>> --- crash-4.0-3.15-org/xendump.h 2006-12-21 09:57:29.000000000 +0900</tt> <tt>> +++ crash-4.0-3.15/xendump.h 2006-12-21 14:37:35.000000000 +0900</tt> <tt>> @@ -113,3 +113,8 @@</tt> <tt>> uint32_t hypercall_imm; /* Break imm for Xen hypercalls. */</tt> <tt>> /* #endif */</tt> <tt>> } xen_domctl_arch_setup_t;</tt> <tt>> +</tt> <tt>> +#ifdef IA64</tt> <tt>> +#define INVALID_MFN (0xffffffffffffffffUL)</tt> <tt>> +#endif</tt> <tt>> +</tt> <tt>> --- crash-4.0-3.15-org/xendump.c 2006-12-21 09:57:29.000000000 +0900</tt> <tt>> +++ crash-4.0-3.15/xendump.c 2006-12-21 14:42:48.000000000 +0900</tt> <tt>> @@ -31,6 +31,7 @@</tt> <tt>></tt> <tt>> static void xc_core_p2m_create(void);</tt> <tt>> static ulong xc_core_pfn_to_page_index(ulong);</tt> <tt>> +static int check_mfn_is_valid(ulong);</tt> <tt>></tt> <tt>> /*</tt> <tt>> * Determine whether a file is a xendump creation, and if TRUE,</tt> <tt>> @@ -1362,7 +1363,7 @@</tt> <tt>> off_t offset;</tt> <tt>></tt> <tt>> if (xd->flags & XC_CORE_NO_P2MM)</tt> <tt>> - return pfn;</tt> <tt>> + return check_mfn_is_valid(pfn) ? pfn : PFN_NOT_FOUND;</tt> <tt>></tt> <tt>> idx = pfn/PFNS_PER_PAGE;</tt> <tt>></tt> <tt>> @@ -1405,6 +1406,34 @@</tt> <tt>> return mfn_idx;</tt> <tt>> }</tt> <tt>></tt> <tt>> +static int</tt> <tt>> +check_mfn_is_valid(ulong idx)</tt> <tt>> +{</tt> <tt>> + ulong mfn;</tt> <tt>> +</tt> <tt>> + if (idx >= xd->xc_core.header.xch_nr_pages)</tt> <tt>> + return FALSE;</tt> <tt>> +</tt> <tt>> + if (lseek(xd->xfd,</tt> <tt>> + (off_t)xd->xc_core.header.xch_index_offset + idx * sizeof(ulong),</tt> <tt>> + SEEK_SET) == -1) {</tt> <tt>> + error(INFO, "cannot lseek to page index\n");</tt> <tt>> + return FALSE;</tt> <tt>> + }</tt> <tt>> +</tt> <tt>> + if (read(xd->xfd, &mfn, sizeof(ulong)) != sizeof(ulong)) {</tt> <tt>> + error(INFO, "cannot read index page %d\n", idx);</tt> <tt>> + return FALSE;</tt> <tt>> + }</tt> <tt>> +</tt> <tt>> + if (mfn == INVALID_MFN) {</tt> <tt>> + error(INFO, "idx: %lx indicates INVALID_MFN\n", idx);</tt> <tt>> + return FALSE;</tt> <tt>> + } else {</tt> <tt>> + return TRUE;</tt> <tt>> + }</tt> <tt>> +}</tt> <tt>> +</tt> <tt>> /*</tt> <tt>> * Store the panic task's stack hooks from where it was found</tt> <tt>> * in get_active_set_panic_task().</tt> <tt>> ---</tt> <tt>></tt> <tt>> Thanks.</tt> <tt>> --</tt> <tt>> Itsuro ODA <oda@valinux.co.jp></tt><tt></tt> <tt>Oda-san,</tt><tt></tt> <tt>Good catch. I was never reading "bad addresses", so I didn't</tt> <tt>see any issue there. Now, after going back and checking it out,</tt> <tt>I see that if you attempt to read an uninstantiated pfn, it returns</tt> <tt>the page index, and so it seeks to and reads whatever's in the</tt> <tt>xendump from that location. Given the way the FV xendump is</tt> <tt>laid out, there has to be *something* there in the dumpfile, which</tt> <tt>I would have thought would have been sparse file space and</tt> <tt>read as all zeroes? But in some dumps, it looks like there's "junk"</tt> <tt>data there? Whatever -- it's harmless.</tt><tt></tt> <tt>In any case, I agree with you -- it's much better to return a read</tt> <tt>error as xc_core_read() expects. I'll just adjust your patch slightly</tt> <tt>to handle all architectures.</tt><tt></tt> <tt>Thanks,</tt> <tt> Dave</tt></blockquote> <tt></tt> <tt>Oda-san,</tt><tt></tt> <tt>Thanks again for the "overnight-QA" and for introducing support</tt> <tt>for xendumps of fully-virtualized ia64 kernels! I've posted a new</tt> <tt>crash version with both fixes.</tt><tt></tt> <tt>I modified your check_mfn_is_valid() function so that it could</tt> <tt>be used by all 3 architectures as well as for xendumps of</tt> <tt>32-bit kernels running on a 64-bit host:</tt> <tt></tt> <tt></tt> <tt>/*</tt> <tt> * In xendumps containing INVALID_MFN markers in the page index,</tt> <tt> * return the validity of the pfn.</tt> <tt> */</tt> <tt>static int</tt> <tt>xc_core_pfn_valid(ulong pfn)</tt> <tt>{</tt> <tt> ulong mfn;</tt> <tt> off_t offset;</tt><tt></tt> <tt> if (pfn >= (ulong)xd->xc_core.header.xch_nr_pages)</tt> <tt> return FALSE;</tt><tt></tt> <tt> offset = (off_t)xd->xc_core.header.xch_index_offset;</tt><tt></tt> <tt> if (xd->flags & XC_CORE_64BIT_HOST)</tt> <tt> offset += (off_t)(pfn * sizeof(ulonglong));</tt> <tt> else</tt> <tt> offset += (off_t)(pfn * sizeof(ulong));</tt><tt></tt> <tt> /*</tt> <tt> * The lseek and read should never fail, so report</tt> <tt> * any errors unconditionally.</tt> <tt> */</tt> <tt> if (lseek(xd->xfd, offset, SEEK_SET) == -1) {</tt> <tt> error(INFO,</tt> <tt> "xendump: cannot lseek to page index for pfn %lx\n",</tt> <tt> pfn);</tt> <tt> return FALSE;</tt> <tt> }</tt><tt></tt> <tt> if (read(xd->xfd, &mfn, sizeof(ulong)) != sizeof(ulong)) {</tt> <tt> error(INFO,</tt> <tt> "xendump: cannot read index page for pfn %lx\n",</tt> <tt> pfn);</tt> <tt> return FALSE;</tt> <tt> }</tt><tt></tt> <tt> /*</tt> <tt> * If it's an invalid mfn, let the caller decide whether</tt> <tt> * to display an error message (unless debugging).</tt> <tt> */</tt> <tt> if (mfn == INVALID_MFN) {</tt> <tt> if (CRASHDEBUG(1))</tt> <tt> error(INFO,</tt> <tt> "xendump: pfn %lx contains INVALID_MFN\n",</tt> <tt> pfn);</tt> <tt> return FALSE;</tt> <tt> }</tt><tt></tt> <tt> return TRUE;</tt> <tt>}</tt><tt></tt> <tt>Thanks again,</tt> <tt> Dave</tt> <tt></tt> </html>