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

[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

> 

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
>
>> 
>