linux-kernel - Re: [RESEND PATCH v8 2/2] ACPI: APEI: handle synchronous exceptions in task work

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: <078b410f-e3b2-0355-d993-40ac46b99870@linux.alibaba.com>
Date:   Tue, 26 Sep 2023 14:38:04 +0800
From:   Shuai Xue <xueshuai@...ux.alibaba.com>
To:     Jarkko Sakkinen <jarkko@...nel.org>, rafael@...nel.org,
        wangkefeng.wang@...wei.com, tanxiaofei@...wei.com,
        mawupeng1@...wei.com, tony.luck@...el.com, linmiaohe@...wei.com,
        naoya.horiguchi@....com, james.morse@....com,
        gregkh@...uxfoundation.org, will@...nel.org
Cc:     linux-acpi@...r.kernel.org, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, akpm@...ux-foundation.org,
        linux-edac@...r.kernel.org, acpica-devel@...ts.linuxfoundation.org,
        stable@...r.kernel.org, x86@...nel.org, justin.he@....com,
        ardb@...nel.org, ying.huang@...el.com, ashish.kalra@....com,
        baolin.wang@...ux.alibaba.com, bp@...en8.de, tglx@...utronix.de,
        mingo@...hat.com, dave.hansen@...ux.intel.com, lenb@...nel.org,
        hpa@...or.com, robert.moore@...el.com, lvying6@...wei.com,
        xiexiuqi@...wei.com, zhuo.song@...ux.alibaba.com
Subject: Re: [RESEND PATCH v8 2/2] ACPI: APEI: handle synchronous exceptions
 in task work



On 2023/9/25 23:00, Jarkko Sakkinen wrote:
> On Tue Sep 19, 2023 at 5:21 AM EEST, Shuai Xue wrote:
>> Hardware errors could be signaled by synchronous interrupt, e.g.  when an
>> error is detected by a background scrubber, or signaled by synchronous
>> exception, e.g. when an uncorrected error is consumed. Both synchronous and
>> asynchronous error are queued and handled by a dedicated kthread in
>> workqueue.
>>
>> commit 7f17b4a121d0 ("ACPI: APEI: Kick the memory_failure() queue for
>> synchronous errors") keep track of whether memory_failure() work was
>> queued, and make task_work pending to flush out the workqueue so that the
>> work for synchronous error is processed before returning to user-space.
>> The trick ensures that the corrupted page is unmapped and poisoned. And
>> after returning to user-space, the task starts at current instruction which
>> triggering a page fault in which kernel will send SIGBUS to current process
>> due to VM_FAULT_HWPOISON.
>>
>> However, the memory failure recovery for hwpoison-aware mechanisms does not
>> work as expected. For example, hwpoison-aware user-space processes like
>> QEMU register their customized SIGBUS handler and enable early kill mode by
>> seting PF_MCE_EARLY at initialization. Then the kernel will directy notify
>> the process by sending a SIGBUS signal in memory failure with wrong
>> si_code: the actual user-space process accessing the corrupt memory
>> location, but its memory failure work is handled in a kthread context, so
>> it will send SIGBUS with BUS_MCEERR_AO si_code to the actual user-space
>> process instead of BUS_MCEERR_AR in kill_proc().
>>
>> To this end, separate synchronous and asynchronous error handling into
>> different paths like X86 platform does:
>>
>> - valid synchronous errors: queue a task_work to synchronously send SIGBUS
>>   before ret_to_user.
>> - valid asynchronous errors: queue a work into workqueue to asynchronously
>>   handle memory failure.
>> - abnormal branches such as invalid PA, unexpected severity, no memory
>>   failure config support, invalid GUID section, OOM, etc.
>>
>> Then for valid synchronous errors, the current context in memory failure is
>> exactly belongs to the task consuming poison data and it will send SIBBUS
>> with proper si_code.
>>
>> Fixes: 7f17b4a121d0 ("ACPI: APEI: Kick the memory_failure() queue for synchronous errors")
>> Signed-off-by: Shuai Xue <xueshuai@...ux.alibaba.com>
>> Tested-by: Ma Wupeng <mawupeng1@...wei.com>
>> Reviewed-by: Kefeng Wang <wangkefeng.wang@...wei.com>
>> Reviewed-by: Xiaofei Tan <tanxiaofei@...wei.com>
>> Reviewed-by: Baolin Wang <baolin.wang@...ux.alibaba.com>
> 
> Did 7f17b4a121d0 actually break something that was not broken before?
> 
> If not, this is (afaik) not a bug fix.

Hi, Jarkko,

It did not. It keeps track of whether memory_failure() work was queued,
and makes task_work pending to flush out the queue. But if no work queued for
synchronous error due to abnormal branches, it does not do a force kill to
current process resulting a hard lockup due to exception loop.

It is fine to me to remove the bug fix tag if you insist on removing it.

Best Regards,
Shuai