linux-kernel - Re: [PATCH v5 16/17] KVM: arm64: Introduce pkvm_dump

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YtqHDTpnn376Qb7u@google.com>
Date:   Fri, 22 Jul 2022 12:16:29 +0100
From:   Oliver Upton <oliver.upton@...ux.dev>
To:     Kalesh Singh <kaleshsingh@...gle.com>
Cc:     maz@...nel.org, mark.rutland@....com, broonie@...nel.org,
        madvenka@...ux.microsoft.com, tabba@...gle.com, will@...nel.org,
        qperret@...gle.com, james.morse@....com, alexandru.elisei@....com,
        suzuki.poulose@....com, catalin.marinas@....com,
        andreyknvl@...il.com, vincenzo.frascino@....com,
        mhiramat@...nel.org, ast@...nel.org, drjones@...hat.com,
        wangkefeng.wang@...wei.com, elver@...gle.com, keirf@...gle.com,
        yuzenghui@...wei.com, ardb@...nel.org,
        linux-arm-kernel@...ts.infradead.org, kvmarm@...ts.cs.columbia.edu,
        linux-kernel@...r.kernel.org, android-mm@...gle.com,
        kernel-team@...roid.com
Subject: Re: [PATCH v5 16/17] KVM: arm64: Introduce pkvm_dump_backtrace()

Hi Kalesh,

On Wed, Jul 20, 2022 at 10:57:27PM -0700, Kalesh Singh wrote:

[...]

> +/*
> + * pkvm_dump_backtrace - Dump the protected nVHE HYP backtrace.
> + *
> + * @hyp_offset: hypervisor offset, used for address translation.
> + *
> + * Dumping of the pKVM HYP backtrace is done by reading the
> + * stack addresses from the shared stacktrace buffer, since the
> + * host cannot direclty access hyperviosr memory in protected
> + * mode.
> + */
> +static void pkvm_dump_backtrace(unsigned long hyp_offset)
> +{
> +	unsigned long *stacktrace_entry
> +		= (unsigned long *)this_cpu_ptr_nvhe_sym(pkvm_stacktrace);
> +	unsigned long va_mask, pc;
> +
> +	va_mask = GENMASK_ULL(vabits_actual - 1, 0);
> +
> +	kvm_err("Protected nVHE HYP call trace:\n");

This and the footer printks should be put in respective helpers to share
between pKVM and non-pKVM backtrace implementations. I imagine users
will invariably bake some pattern matching to scrape traces, and it
should be consistent between both flavors.

> +	/* The stack trace is terminated by a null entry */
> +	for (; *stacktrace_entry; stacktrace_entry++) {

At the point we're dumping the backtrace we know that EL2 has already
soiled itself, so we shouldn't explicitly depend on it providing NULL
terminators. I believe this loop should have an explicit range && NULL
check.

--
Thanks,
Oliver