Re: [PATCH v2 1/2] RAS: Fix inverted context info bounds check in ARM processor errors

Re: [PATCH v2 1/2] RAS: Fix inverted context info bounds check in ARM processor errors

[Liuwenliang (Abbott Liu)]

Hi Hanjun, thinks for your review.
>Hi Abbott,
>
>On 2026/8/25 21:43, Abbott Liu wrote:
>> Commit 87880af2d24e ("APEI/GHES: ARM processor Error: don't go past
>> allocated memory") added bounds checks for malformed ARM processor
>> error records but contained a bug:
>> 
>>    In log_arm_hw_error(), the ctx_info bounds check is inverted. The
>>    condition `sz + (long)ctx_info - (long)err >= err->section_length`
>>    adds ctx_info->size when the context header is already past the end
>>    of the section instead of when it is within bounds. So change the
>>    comparison to <=.
>> 
>> Fixes: 87880af2d24e ("APEI/GHES: ARM processor Error: don't go past allocated memory")
>> 
>
>This empty line is not needed.

I am very sorry for making such a basic mistake; this issue will be resolved
in the next version.

>
>> Signed-off-by: Abbott Liu <liuwenliang@huawei.com>
>> ---
>>   drivers/ras/ras.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/ras/ras.c b/drivers/ras/ras.c
>> index 03df3db62334..2540538a16a8 100644
>> --- a/drivers/ras/ras.c
>> +++ b/drivers/ras/ras.c
>> @@ -74,7 +74,7 @@ void log_arm_hw_error(struct cper_sec_proc_arm *err, const u8 sev)
>>   	for (n = 0; n < err->context_info_num; n++) {
>>   		sz = sizeof(struct cper_arm_ctx_info);
>>   
>> -		if (sz + (long)ctx_info - (long)err >= err->section_length)
>> +		if (sz + (long)ctx_info - (long)err <= err->section_length)
>>   			sz += ctx_info->size;
>
>sz is an int and ctx_info->size is u32, if ctx_info->size is big enough
>for example over 0x7fffffff, the sz will be negative.
>
>>   
>>   		ctx_info = (struct cper_arm_ctx_info *)((long)ctx_info + sz);
>
>if the sz is negative, the ctx_info may pointer to a wrong place.
>
>ctx_info->size will not over 0x7fffffff in practical but should we
>consider the overflows?

I also think that it won't actually happen in practice, so there
isn't much need to consider this overflow issue. But even if we do
consider it, the code modifications wouldn't be too complex. I plan
to fix it in the next version.

>
>Thanks
>Hanjun
>
>> 
>

[PATCH v2 0/2] RAS: Fix ARM processor error bounds checking

[Abbott Liu]

Commit 87880af2d24e ("APEI/GHES: ARM processor Error: don't go past
allocated memory") and commit 05954511b73e ("RAS: Report all ARM
processor CPER information to userspace") introduced bounds checking
for malformed ARM processor error records in log_arm_hw_error().
However, two issues remain:

1. The ctx_info bounds check condition is inverted: it adds
   ctx_info->size when the context header is already past the end of
   the section instead of when it is within bounds.

2. When vsei_len < 0, the error path does not verify pei_len and
   ctx_len. Since these are derived from err_info_num and
   context_info_num, they may describe regions beyond the allocated
   record, causing trace_arm_event() to read out of bounds.

Patch 1 fixes the inverted bounds check for context info iteration.
Patch 2 sanitizes pei_len/ctx_len/ven_err_data in the vsei_len < 0
error path to prevent out-of-bounds reads in trace_arm_event().

Changes in v2:
  - Split the original single patch into two separate patches, one
    for each distinct fix, to ease review and backporting.

v1:
  RAS: Fix out-of-range section_length in ARM processor error handling
  https://lore.kernel.org/all/20260820131829.1006371-1-liuwenliang@huawei.com/

Abbott Liu (2):
  RAS: Fix inverted context info bounds check in ARM processor errors
  RAS: Fix out-of-bounds read when tracing arm_event

 drivers/ras/ras.c | 33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)

-- 
2.43.0

[PATCH v2 1/2] RAS: Fix inverted context info bounds check in ARM processor errors

[Abbott Liu]

Commit 87880af2d24e ("APEI/GHES: ARM processor Error: don't go past
allocated memory") added bounds checks for malformed ARM processor
error records but contained a bug:

  In log_arm_hw_error(), the ctx_info bounds check is inverted. The
  condition `sz + (long)ctx_info - (long)err >= err->section_length`
  adds ctx_info->size when the context header is already past the end
  of the section instead of when it is within bounds. So change the
  comparison to <=.

Fixes: 87880af2d24e ("APEI/GHES: ARM processor Error: don't go past allocated memory")

Signed-off-by: Abbott Liu <liuwenliang@huawei.com>
---
 drivers/ras/ras.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ras/ras.c b/drivers/ras/ras.c
index 03df3db62334..2540538a16a8 100644
--- a/drivers/ras/ras.c
+++ b/drivers/ras/ras.c
@@ -74,7 +74,7 @@ void log_arm_hw_error(struct cper_sec_proc_arm *err, const u8 sev)
 	for (n = 0; n < err->context_info_num; n++) {
 		sz = sizeof(struct cper_arm_ctx_info);
 
-		if (sz + (long)ctx_info - (long)err >= err->section_length)
+		if (sz + (long)ctx_info - (long)err <= err->section_length)
 			sz += ctx_info->size;
 
 		ctx_info = (struct cper_arm_ctx_info *)((long)ctx_info + sz);
-- 
2.43.0

Re: [PATCH v2 1/2] RAS: Fix inverted context info bounds check in ARM processor errors

[Hanjun Guo]

Hi Abbott,

On 2026/8/25 21:43, Abbott Liu wrote:
> Commit 87880af2d24e ("APEI/GHES: ARM processor Error: don't go past
> allocated memory") added bounds checks for malformed ARM processor
> error records but contained a bug:
> 
>    In log_arm_hw_error(), the ctx_info bounds check is inverted. The
>    condition `sz + (long)ctx_info - (long)err >= err->section_length`
>    adds ctx_info->size when the context header is already past the end
>    of the section instead of when it is within bounds. So change the
>    comparison to <=.
> 
> Fixes: 87880af2d24e ("APEI/GHES: ARM processor Error: don't go past allocated memory")
> 

This empty line is not needed.

> Signed-off-by: Abbott Liu <liuwenliang@huawei.com>
> ---
>   drivers/ras/ras.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/ras/ras.c b/drivers/ras/ras.c
> index 03df3db62334..2540538a16a8 100644
> --- a/drivers/ras/ras.c
> +++ b/drivers/ras/ras.c
> @@ -74,7 +74,7 @@ void log_arm_hw_error(struct cper_sec_proc_arm *err, const u8 sev)
>   	for (n = 0; n < err->context_info_num; n++) {
>   		sz = sizeof(struct cper_arm_ctx_info);
>   
> -		if (sz + (long)ctx_info - (long)err >= err->section_length)
> +		if (sz + (long)ctx_info - (long)err <= err->section_length)
>   			sz += ctx_info->size;

sz is an int and ctx_info->size is u32, if ctx_info->size is big enough
for example over 0x7fffffff, the sz will be negative.

>   
>   		ctx_info = (struct cper_arm_ctx_info *)((long)ctx_info + sz);

if the sz is negative, the ctx_info may pointer to a wrong place.

ctx_info->size will not over 0x7fffffff in practical but should we
consider the overflows?

Thanks
Hanjun

>