Itsuro ODA wrote:
> Hi Dave,
>
> > - Introduced support for xendumps of para-virtualized ia64 kernels.
> > It should be noted that currently the ia64 Xen kernel does not
> > lay down a switch_stack for the panic task, so only raw "bt -t"
> > backtraces can be done on the panic task. (anderson@xxxxxxxxxx)
>
> page_index may contain INVALID_MFN (0xffffffffffffffffUL).
> I think it is better to check it.
> Here is a patch for this.
>
> ---
> --- crash-4.0-3.15-org/xendump.h 2006-12-21 09:57:29.000000000 +0900
> +++ crash-4.0-3.15/xendump.h 2006-12-21 14:37:35.000000000 +0900
> @@ -113,3 +113,8 @@
> uint32_t hypercall_imm; /* Break imm for Xen hypercalls. */
> /* #endif */
> } xen_domctl_arch_setup_t;
> +
> +#ifdef IA64
> +#define INVALID_MFN (0xffffffffffffffffUL)
> +#endif
> +
> --- crash-4.0-3.15-org/xendump.c 2006-12-21 09:57:29.000000000 +0900
> +++ crash-4.0-3.15/xendump.c 2006-12-21 14:42:48.000000000 +0900
> @@ -31,6 +31,7 @@
>
> static void xc_core_p2m_create(void);
> static ulong xc_core_pfn_to_page_index(ulong);
> +static int check_mfn_is_valid(ulong);
>
> /*
> * Determine whether a file is a xendump creation, and if TRUE,
> @@ -1362,7 +1363,7 @@
> off_t offset;
>
> if (xd->flags & XC_CORE_NO_P2MM)
> - return pfn;
> + return check_mfn_is_valid(pfn) ? pfn : PFN_NOT_FOUND;
>
> idx = pfn/PFNS_PER_PAGE;
>
> @@ -1405,6 +1406,34 @@
> return mfn_idx;
> }
>
> +static int
> +check_mfn_is_valid(ulong idx)
> +{
> + ulong mfn;
> +
> + if (idx >= xd->xc_core.header.xch_nr_pages)
> + return FALSE;
> +
> + if (lseek(xd->xfd,
> + (off_t)xd->xc_core.header.xch_index_offset + idx * sizeof(ulong),
> + SEEK_SET) == -1) {
> + error(INFO, "cannot lseek to page index\n");
> + return FALSE;
> + }
> +
> + if (read(xd->xfd, &mfn, sizeof(ulong)) != sizeof(ulong)) {
> + error(INFO, "cannot read index page %d\n", idx);
> + return FALSE;
> + }
> +
> + if (mfn == INVALID_MFN) {
> + error(INFO, "idx: %lx indicates INVALID_MFN\n", idx);
> + return FALSE;
> + } else {
> + return TRUE;
> + }
> +}
> +
> /*
> * Store the panic task's stack hooks from where it was found
> * in get_active_set_panic_task().
> ---
>
> Thanks.
> --
> Itsuro ODA <oda@xxxxxxxxxxxxx>
Oda-san,
Good catch. I was never reading "bad addresses", so I didn't
see any issue there. Now, after going back and checking it out,
I see that if you attempt to read an uninstantiated pfn, it returns
the page index, and so it seeks to and reads whatever's in the
xendump from that location. Given the way the FV xendump is
laid out, there has to be *something* there in the dumpfile, which
I would have thought would have been sparse file space and
read as all zeroes? But in some dumps, it looks like there's "junk"
data there? Whatever -- it's harmless.
In any case, I agree with you -- it's much better to return a read
error as xc_core_read() expects. I'll just adjust your patch slightly
to handle all architectures.
Thanks,
Dave
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/crash-utility
