Re: [PATCH v2 1/2] apei/ghes: don't go past the ARM processor CPER record buffer

Re: [PATCH v2 1/2] apei/ghes: don't go past the ARM processor CPER record buffer

[Jonathan Cameron]

On Fri, 28 Nov 2025 11:53:00 +0100
Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote:

> There's a logic inside ghes/cper to detect if the section_length
> is too small, but it doesn't detect if it is too big.
> 
> Currently, if the firmware receives an ARM processor CPER record
> stating that a section length is big, kernel will blindly trust
> section_lentgh, producing a very long dump. For instance, a 67

section_length

> bytes record with ERR_INFO_NUM set 46198 and section length
> set to 854918320 would dump a lot of data going a way past the
> firmware memory-mapped area.
> 
> Fix it by adding a logic to prevent it to go past the buffer
> if ERR_INFO_NUM is too big, making it report instead:
> 
> 	[Hardware Error]: Hardware error from APEI Generic Hardware Error Source: 1
> 	[Hardware Error]: event severity: recoverable
> 	[Hardware Error]:  Error 0, type: recoverable
> 	[Hardware Error]:   section_type: ARM processor error
> 	[Hardware Error]:   MIDR: 0xff304b2f8476870a
> 	[Hardware Error]:   section length: 854918320, CPER size: 67
> 	[Hardware Error]:   section length is too big
> 	[Hardware Error]:   firmware-generated error record is incorrect
> 	[Hardware Error]:   ERR_INFO_NUM is 46198
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---
>  drivers/acpi/apei/ghes.c        | 13 +++++++++++++
>  drivers/firmware/efi/cper-arm.c | 14 +++++++++-----
>  drivers/firmware/efi/cper.c     |  3 ++-
>  include/linux/cper.h            |  3 ++-
>  4 files changed, 26 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index 56107aa00274..8b90b6f3e866 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -557,6 +557,7 @@ static bool ghes_handle_arm_hw_error(struct acpi_hest_generic_data *gdata,
>  {
>  	struct cper_sec_proc_arm *err = acpi_hest_get_payload(gdata);
>  	int flags = sync ? MF_ACTION_REQUIRED : 0;
> +	int length = gdata->error_data_length;
>  	char error_type[120];
>  	bool queued = false;
>  	int sec_sev, i;
> @@ -568,7 +569,12 @@ static bool ghes_handle_arm_hw_error(struct acpi_hest_generic_data *gdata,
>  		return false;
>  
>  	p = (char *)(err + 1);
> +	length -= sizeof(err);
> +
>  	for (i = 0; i < err->err_info_num; i++) {
> +		if (length <= 0)
> +			break;
> +
>  		struct cper_arm_err_info *err_info = (struct cper_arm_err_info *)p;
>  		bool is_cache = err_info->type & CPER_ARM_CACHE_ERROR;
>  		bool has_pa = (err_info->validation_bits & CPER_ARM_INFO_VALID_PHYSICAL_ADDR);
> @@ -580,10 +586,17 @@ static bool ghes_handle_arm_hw_error(struct acpi_hest_generic_data *gdata,
>  		 * and don't filter out 'corrected' error here.
>  		 */
>  		if (is_cache && has_pa) {
> +			length -= err_info->length;
> +			if (length < 0)
> +				break;
>  			queued = ghes_do_memory_failure(err_info->physical_fault_addr, flags);
>  			p += err_info->length;
> +
>  			continue;
>  		}
> +		length -= err_info->length;
> +			if (length < 0)
Indent odd.

> +				break;
>  
>  		cper_bits_to_str(error_type, sizeof(error_type),
>  				 FIELD_GET(CPER_ARM_ERR_TYPE_MASK, err_info->type),

Re: [PATCH v2 1/2] apei/ghes: don't go past the ARM processor CPER record buffer

[Shuai Xue]

在 2025/11/28 18:53, Mauro Carvalho Chehab 写道:
> There's a logic inside ghes/cper to detect if the section_length
> is too small, but it doesn't detect if it is too big.
> 
> Currently, if the firmware receives an ARM processor CPER record
> stating that a section length is big, kernel will blindly trust
> section_lentgh, producing a very long dump. For instance, a 67
> bytes record with ERR_INFO_NUM set 46198 and section length
> set to 854918320 would dump a lot of data going a way past the
> firmware memory-mapped area.
> 
> Fix it by adding a logic to prevent it to go past the buffer
> if ERR_INFO_NUM is too big, making it report instead:
> 
> 	[Hardware Error]: Hardware error from APEI Generic Hardware Error Source: 1
> 	[Hardware Error]: event severity: recoverable
> 	[Hardware Error]:  Error 0, type: recoverable
> 	[Hardware Error]:   section_type: ARM processor error
> 	[Hardware Error]:   MIDR: 0xff304b2f8476870a
> 	[Hardware Error]:   section length: 854918320, CPER size: 67
> 	[Hardware Error]:   section length is too big
> 	[Hardware Error]:   firmware-generated error record is incorrect
> 	[Hardware Error]:   ERR_INFO_NUM is 46198
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---
>   drivers/acpi/apei/ghes.c        | 13 +++++++++++++
>   drivers/firmware/efi/cper-arm.c | 14 +++++++++-----
>   drivers/firmware/efi/cper.c     |  3 ++-
>   include/linux/cper.h            |  3 ++-
>   4 files changed, 26 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index 56107aa00274..8b90b6f3e866 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -557,6 +557,7 @@ static bool ghes_handle_arm_hw_error(struct acpi_hest_generic_data *gdata,
>   {
>   	struct cper_sec_proc_arm *err = acpi_hest_get_payload(gdata);
>   	int flags = sync ? MF_ACTION_REQUIRED : 0;
> +	int length = gdata->error_data_length;
>   	char error_type[120];
>   	bool queued = false;
>   	int sec_sev, i;
> @@ -568,7 +569,12 @@ static bool ghes_handle_arm_hw_error(struct acpi_hest_generic_data *gdata,
>   		return false;
>   
>   	p = (char *)(err + 1);
> +	length -= sizeof(err);
> +
>   	for (i = 0; i < err->err_info_num; i++) {
> +		if (length <= 0)
> +			break;
> +

Hi, Mauro,

The bounds checking logic is duplicated - it appears both in the cache
error handling branch and after it. This could be simplified. It would
be better to ensure we have enough data for the error info header in one
check.

		/* Ensure we have enough data for the error info header */
		if (length < sizeof(struct cper_arm_err_info))
			break;

And it would be better to validate the claimed length before using it.

		/* Validate the claimed length before using it */
		if (err_info->length < sizeof(struct cper_arm_err_info) ||
		    err_info->length > length)
			break;

Thanks.
Shuai

[PATCH v2 1/2] apei/ghes: don't go past the ARM processor CPER record buffer

[Mauro Carvalho Chehab]

There's a logic inside ghes/cper to detect if the section_length
is too small, but it doesn't detect if it is too big.

Currently, if the firmware receives an ARM processor CPER record
like:

  00000000  01 c2 bf 6e 76 b4 94 d7 b0 04 f5 32 e3 5e 89 23   ...nv......2.^.#
  00000010  f6 4f 40 31 fd 70 3f 7c 0a 87 76 84 2f 4b 30 ff   .O@1.p?|..v./K0.
  00000020  24 20 71 0d c0 92 4b 0f ae 9b 94 9d 78 8a 7f 6b   $ q...K.....x..k
  00000030  89 2f 10 8a 6a bf f4 01 96 12 b0 90 b3 9a 08 33   ./..j..........3
  00000040  0d 01 61                                          ..a

It would produce a very long record, going past the buffer, as,
in the above record, ERR_INFO_NUM is set to 46198.

Fix it by adding a logic to prevent it to go past the buffer
if ERR_INFO_NUM is too big, reporting instead:

	[Hardware Error]: Hardware error from APEI Generic Hardware Error Source: 1
	[Hardware Error]: event severity: recoverable
	[Hardware Error]:  Error 0, type: recoverable
	[Hardware Error]:   section_type: ARM processor error
	[Hardware Error]:   MIDR: 0xff304b2f8476870a
	[Hardware Error]:   section length: 854918320, CPER size: 67
	[Hardware Error]:   section length is too big
	[Hardware Error]:   firmware-generated error record is incorrect
	[Hardware Error]:   ERR_INFO_NUM is 46198

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/acpi/apei/ghes.c        | 13 +++++++++++++
 drivers/firmware/efi/cper-arm.c | 14 +++++++++-----
 drivers/firmware/efi/cper.c     |  3 ++-
 include/linux/cper.h            |  3 ++-
 4 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 56107aa00274..8b90b6f3e866 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -557,6 +557,7 @@ static bool ghes_handle_arm_hw_error(struct acpi_hest_generic_data *gdata,
 {
 	struct cper_sec_proc_arm *err = acpi_hest_get_payload(gdata);
 	int flags = sync ? MF_ACTION_REQUIRED : 0;
+	int length = gdata->error_data_length;
 	char error_type[120];
 	bool queued = false;
 	int sec_sev, i;
@@ -568,7 +569,12 @@ static bool ghes_handle_arm_hw_error(struct acpi_hest_generic_data *gdata,
 		return false;
 
 	p = (char *)(err + 1);
+	length -= sizeof(err);
+
 	for (i = 0; i < err->err_info_num; i++) {
+		if (length <= 0)
+			break;
+
 		struct cper_arm_err_info *err_info = (struct cper_arm_err_info *)p;
 		bool is_cache = err_info->type & CPER_ARM_CACHE_ERROR;
 		bool has_pa = (err_info->validation_bits & CPER_ARM_INFO_VALID_PHYSICAL_ADDR);
@@ -580,10 +586,17 @@ static bool ghes_handle_arm_hw_error(struct acpi_hest_generic_data *gdata,
 		 * and don't filter out 'corrected' error here.
 		 */
 		if (is_cache && has_pa) {
+			length -= err_info->length;
+			if (length < 0)
+				break;
 			queued = ghes_do_memory_failure(err_info->physical_fault_addr, flags);
 			p += err_info->length;
+
 			continue;
 		}
+		length -= err_info->length;
+			if (length < 0)
+				break;
 
 		cper_bits_to_str(error_type, sizeof(error_type),
 				 FIELD_GET(CPER_ARM_ERR_TYPE_MASK, err_info->type),
diff --git a/drivers/firmware/efi/cper-arm.c b/drivers/firmware/efi/cper-arm.c
index 76542a53e202..6fe26abc9c11 100644
--- a/drivers/firmware/efi/cper-arm.c
+++ b/drivers/firmware/efi/cper-arm.c
@@ -226,9 +226,10 @@ static void cper_print_arm_err_info(const char *pfx, u32 type,
 }
 
 void cper_print_proc_arm(const char *pfx,
-			 const struct cper_sec_proc_arm *proc)
+			 const struct cper_sec_proc_arm *proc,
+			 u32 length)
 {
-	int i, len, max_ctx_type;
+	int len, i, max_ctx_type;
 	struct cper_arm_err_info *err_info;
 	struct cper_arm_ctx_info *ctx_info;
 	char newpfx[64], infopfx[ARRAY_SIZE(newpfx) + 1];
@@ -238,9 +239,12 @@ void cper_print_proc_arm(const char *pfx,
 
 	len = proc->section_length - (sizeof(*proc) +
 		proc->err_info_num * (sizeof(*err_info)));
-	if (len < 0) {
-		printk("%ssection length: %d\n", pfx, proc->section_length);
-		printk("%ssection length is too small\n", pfx);
+
+	if (len < 0 || len > length) {
+		printk("%ssection length: %d, CPER size: %d\n",
+		       pfx, proc->section_length, length);
+		printk("%ssection length is too %s\n", pfx,
+		       (len > length) ? "big" : "small");
 		printk("%sfirmware-generated error record is incorrect\n", pfx);
 		printk("%sERR_INFO_NUM is %d\n", pfx, proc->err_info_num);
 		return;
diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
index 0232bd040f61..88fc0293f876 100644
--- a/drivers/firmware/efi/cper.c
+++ b/drivers/firmware/efi/cper.c
@@ -659,7 +659,8 @@ cper_estatus_print_section(const char *pfx, struct acpi_hest_generic_data *gdata
 
 		printk("%ssection_type: ARM processor error\n", newpfx);
 		if (gdata->error_data_length >= sizeof(*arm_err))
-			cper_print_proc_arm(newpfx, arm_err);
+			cper_print_proc_arm(newpfx, arm_err,
+					    gdata->error_data_length);
 		else
 			goto err_section_too_small;
 #endif
diff --git a/include/linux/cper.h b/include/linux/cper.h
index 5b1236d8c65b..440b35e459e5 100644
--- a/include/linux/cper.h
+++ b/include/linux/cper.h
@@ -595,7 +595,8 @@ void cper_mem_err_pack(const struct cper_sec_mem_err *,
 const char *cper_mem_err_unpack(struct trace_seq *,
 				struct cper_mem_err_compact *);
 void cper_print_proc_arm(const char *pfx,
-			 const struct cper_sec_proc_arm *proc);
+			 const struct cper_sec_proc_arm *proc,
+			 u32 length);
 void cper_print_proc_ia(const char *pfx,
 			const struct cper_sec_proc_ia *proc);
 int cper_mem_err_location(struct cper_mem_err_compact *mem, char *msg);
-- 
2.52.0