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]
Date:   Sat, 25 Nov 2023 14:44:52 +0800
From:   Shuai Xue <>
To:     Borislav Petkov <>
Subject: Re: [PATCH v9 0/2] ACPI: APEI: handle synchronous errors in task work
 with proper si_code

On 2023/11/23 23:07, Borislav Petkov wrote:

Hi, Borislav,

Thank you for your reply and advice.

> On Sat, Oct 07, 2023 at 03:28:16PM +0800, Shuai Xue wrote:
>> However, this trick is not always be effective
> So far so good.
> What's missing here is why "this trick" is not always effective.

> Basically to explain what exactly the problem is.

I think the main point is that this trick for AR error is not effective,

- an AR error consumed by current process is deferred to handle in a
  dedicated kernel thread, but memory_failure() assumes that it runs in the
  current context
- another page fault is not unnecessary, we can send sigbus to current
  process in the first Synchronous External Abort SEA on arm64 (analogy
  Machine Check Exception on x86)

>> For example, hwpoison-aware user-space processes use the si_code:
>> BUS_MCEERR_AO for 'action optional' early notifications, and BUS_MCEERR_AR
>> for 'action required' synchronous/late notifications. Specifically, when a
>> signal with SIGBUS_MCEERR_AR is delivered to QEMU, it will inject a vSEA to
>> Guest kernel. In contrast, a signal with SIGBUS_MCEERR_AO will be ignored
>> by QEMU.[1]
>> Fix it by seting memory failure flags as MF_ACTION_REQUIRED on synchronous events. (PATCH 1)
> So you're fixing qemu by "fixing" the kernel?
> This doesn't make any sense.

I just give an example that the user space process *really* relys on the
si_code of signal to handle hardware errors

> Make errors which are ACPI_HEST_NOTIFY_SEA type return
> MF_ACTION_REQUIRED so that it *happens* to fix your use case.
> Sounds like a lot of nonsense to me.
> What is the issue here you're trying to solve?

The SIGBUS si_codes defined in include/uapi/asm-generic/siginfo.h says:

    /* hardware memory error consumed on a machine check: action required */
    #define BUS_MCEERR_AR	4
    /* hardware memory error detected in process but not consumed: action optional*/
    #define BUS_MCEERR_AO	5

When a synchronous error is consumed by Guest, the kernel should send a
signal with BUS_MCEERR_AR instead of BUS_MCEERR_AO.

>> 2. Handle memory_failure() abnormal fails to avoid a unnecessary reboot
>> If process mapping fault page, but memory_failure() abnormal return before
>> try_to_unmap(), for example, the fault page process mapping is KSM page.
>> In this case, arm64 cannot use the page fault process to terminate the
>> synchronous exception loop.[4]
>> This loop can potentially exceed the platform firmware threshold or even trigger
>> a kernel hard lockup, leading to a system reboot. However, kernel has the
>> capability to recover from this error.
>> Fix it by performing a force kill when memory_failure() abnormal fails or when
>> other abnormal synchronous errors occur.
> Just like that?
> Without giving the process the opportunity to even save its other data?


> So this all is still very confusing, patches definitely need splitting
> and this whole thing needs restraint.
> You go and do this: you split *each* issue you're addressing into
> a separate patch and explain it like this:
> ---
> 1. Prepare the context for the explanation briefly.
> 2. Explain the problem at hand.
> 3. "It happens because of <...>"
> 4. "Fix it by doing X"
> 5. "(Potentially do Y)."
> ---
> and each patch explains *exactly* *one* issue, what happens, why it
> happens and just the fix for it and *why* it is needed.
> Otherwise, this is unreviewable.

Thank you for your valuable suggestion, I will split the patches and
resubmit a new patch set.

> Thx.

Best Regards,

Powered by blists - more mailing lists