diff mbox series

[v14,1/3] ACPI: APEI: send SIGBUS to current task if synchronous memory error not recovered

Message ID 20241014084240.18614-2-xueshuai@linux.alibaba.com (mailing list archive)
State New
Headers show
Series [v14,1/3] ACPI: APEI: send SIGBUS to current task if synchronous memory error not recovered | expand

Commit Message

Shuai Xue Oct. 14, 2024, 8:42 a.m. UTC
Synchronous error was detected as a result of user-space process accessing
a 2-bit uncorrected error. The CPU will take a synchronous error exception
such as Synchronous External Abort (SEA) on Arm64. The kernel will queue a
memory_failure() work which poisons the related page, unmaps the page, and
then sends a SIGBUS to the process, so that a system wide panic can be
avoided.

However, no memory_failure() work will be queued when abnormal synchronous
errors occur. These errors can include situations such as invalid PA,
unexpected severity, no memory failure config support, invalid GUID
section, etc. In such case, the user-space process will trigger SEA again.
This loop can potentially exceed the platform firmware threshold or even
trigger a kernel hard lockup, leading to a system reboot.

Fix it by performing a force kill if no memory_failure() work is queued
for synchronous errors.

Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 drivers/acpi/apei/ghes.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Jonathan Cameron Oct. 17, 2024, 9:39 a.m. UTC | #1
On Mon, 14 Oct 2024 16:42:38 +0800
Shuai Xue <xueshuai@linux.alibaba.com> wrote:

> Synchronous error was detected as a result of user-space process accessing
> a 2-bit uncorrected error. The CPU will take a synchronous error exception
> such as Synchronous External Abort (SEA) on Arm64. The kernel will queue a
> memory_failure() work which poisons the related page, unmaps the page, and
> then sends a SIGBUS to the process, so that a system wide panic can be
> avoided.
> 
> However, no memory_failure() work will be queued when abnormal synchronous
> errors occur. These errors can include situations such as invalid PA,
> unexpected severity, no memory failure config support, invalid GUID
> section, etc. In such case, the user-space process will trigger SEA again.
> This loop can potentially exceed the platform firmware threshold or even
> trigger a kernel hard lockup, leading to a system reboot.
> 
> Fix it by performing a force kill if no memory_failure() work is queued
> for synchronous errors.
> 
> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

The subtle cases in here are the various other forms of delayed handling
buried in some of the record handling that don't set queued.
I've been through them all and have convinced myself that either 
hey should never be synchronous or that there is no attempt to
recover in kernel today (non memory things such as CXL protocol
collapse, which might I guess be detected synchronously on a read
- though I'd expect poison and a memory error first) so the correct
thing to do is what you have here.

Fiddly code though with a lot of paths, so more eyes welcome!

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

+CC linux-cxl for info.
 
> ---
>  drivers/acpi/apei/ghes.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index ada93cfde9ba..f2ee28c44d7a 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -801,6 +801,16 @@ static bool ghes_do_proc(struct ghes *ghes,
>  		}
>  	}
>  
> +	/*
> +	 * If no memory failure work is queued for abnormal synchronous
> +	 * errors, do a force kill.
> +	 */
> +	if (sync && !queued) {
> +		pr_err("%s:%d: hardware memory corruption (SIGBUS)\n",
> +			current->comm, task_pid_nr(current));
> +		force_sig(SIGBUS);
> +	}
> +
>  	return queued;
>  }
>
Shuai Xue Oct. 17, 2024, 11:41 p.m. UTC | #2
在 2024/10/17 17:39, Jonathan Cameron 写道:
> On Mon, 14 Oct 2024 16:42:38 +0800
> Shuai Xue <xueshuai@linux.alibaba.com> wrote:
> 
>> Synchronous error was detected as a result of user-space process accessing
>> a 2-bit uncorrected error. The CPU will take a synchronous error exception
>> such as Synchronous External Abort (SEA) on Arm64. The kernel will queue a
>> memory_failure() work which poisons the related page, unmaps the page, and
>> then sends a SIGBUS to the process, so that a system wide panic can be
>> avoided.
>>
>> However, no memory_failure() work will be queued when abnormal synchronous
>> errors occur. These errors can include situations such as invalid PA,
>> unexpected severity, no memory failure config support, invalid GUID
>> section, etc. In such case, the user-space process will trigger SEA again.
>> This loop can potentially exceed the platform firmware threshold or even
>> trigger a kernel hard lockup, leading to a system reboot.
>>
>> Fix it by performing a force kill if no memory_failure() work is queued
>> for synchronous errors.
>>
>> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
>> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> The subtle cases in here are the various other forms of delayed handling
> buried in some of the record handling that don't set queued.
> I've been through them all and have convinced myself that either
> hey should never be synchronous or that there is no attempt to
> recover in kernel today (non memory things such as CXL protocol
> collapse, which might I guess be detected synchronously on a read
> - though I'd expect poison and a memory error first) so the correct
> thing to do is what you have here.
> 
> Fiddly code though with a lot of paths, so more eyes welcome!
> 
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> +CC linux-cxl for info.

Thanks :)

Best Regards,
Shuai
diff mbox series

Patch

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index ada93cfde9ba..f2ee28c44d7a 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -801,6 +801,16 @@  static bool ghes_do_proc(struct ghes *ghes,
 		}
 	}
 
+	/*
+	 * If no memory failure work is queued for abnormal synchronous
+	 * errors, do a force kill.
+	 */
+	if (sync && !queued) {
+		pr_err("%s:%d: hardware memory corruption (SIGBUS)\n",
+			current->comm, task_pid_nr(current));
+		force_sig(SIGBUS);
+	}
+
 	return queued;
 }