Re: [Crash-utility] [PATCH] x86_64: Fix check for __per_cpu_offset initialisation
Hi Lianbo,
On Wed, 11 Aug 2021 17:05:26 +0800
lijiang <lijiang(a)redhat.com> wrote:
>
> Date: Thu, 5 Aug 2021 15:19:37 +0200
> From: Philipp Rudo <prudo(a)redhat.com>
> To: crash-utility(a)redhat.com
> Subject: [Crash-utility] [PATCH] x86_64: Fix check for
> __per_cpu_offset initialisation
> Message-ID: <20210805131937.5051-1-prudo(a)redhat.com>
>
> Since at least kernel v2.6.30 the __per_cpu_offset gets initialized to
> __per_cpu_load. So first check if the __per_cpu_offset was set to a
> proper value before reading any per cpu variable to prevent potential
> bugs.
>
>
Hi, Philipp
Thank you for the patch. Can you help to describe more details about the
potential risks? and what conditions might trigger the potential bugs?
the bug is always triggered during initialization of the per-cpu data
on x86_64. At least for kernels not using struct x8664_pda, which
AFAIK was also removed with kernel v2.6.30.
The risk for crash is low. Right after the superfluous read there is a
check if the read cpunumber matches the expected one.
if (cpunumber != cpus)
break;
So the worst case scenario I see is that crash initializes one
additional cpu with non-sense data. But given that the bug exists for
~12 years and nobody reported such an bug I assume that the check worked
well so far.
Did you mean that it's related to the crash live analysis
issue(1978032)? I
tried to reproduce it, but so far I haven't reproduced it with the upstream
kernel.
Yes, this bug is related to bz1978032. For whatever reason the
superfluous read triggered the panic.
I could reproduce the bug upstream with CONFIG_IO_URING _disabled_.
Unfortunately there is a RHEL-only patch [1] that tampers with the
Kconfig for IO_URING. So when you copy a kernel-ark config to the
upstream repo and run 'make oldconfig' the IO_URING will silently be
_enabled_.
BTW, I tried to reproduce the panic yesterday on kernel-5.14.0-0.rc4
but failed. Not sure if the bug was fixed in the meantime or I was
simply "lucky"...
Thanks
Philipp
[1]
https://gitlab.com/cki-project/kernel-ark/-/commit/50f8c7fb77c97617dbf325...
Thanks.
Lianbo
> Signed-off-by: Philipp Rudo <prudo(a)redhat.com>
> ---
> x86_64.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/x86_64.c b/x86_64.c
> index 6eb7d67..0bb8705 100644
> --- a/x86_64.c
> +++ b/x86_64.c
> @@ -1327,6 +1327,8 @@ x86_64_per_cpu_init(void)
> ms->stkinfo.isize = 16384;
>
> for (i = cpus = 0; i < NR_CPUS; i++) {
> + if (kt->__per_cpu_offset[i] ==
> symbol_value("__per_cpu_load"))
> + break;
> if (!readmem(cpu_sp->value + kt->__per_cpu_offset[i],
> KVADDR, &cpunumber, sizeof(int),
> "cpu number (per_cpu)", QUIET|RETURN_ON_ERROR))
> @@ -5602,7 +5604,7 @@ x86_64_get_smp_cpus(void)
> return 1;
>
> for (i = cpus = 0; i < NR_CPUS; i++) {
> - if (kt->__per_cpu_offset[i] == 0)
> + if (kt->__per_cpu_offset[i] ==
> symbol_value("__per_cpu_load"))
> break;
> if (!readmem(sp->value + kt->__per_cpu_offset[i],
> KVADDR, &cpunumber, sizeof(int),
> --
> 2.31.1
>
>