[PATCH] scs: Fix a wrong parameter in __scs_magic

[PATCH] scs: Fix a wrong parameter in __scs_magic

[Zhichi Lin]

__scs_magic() needs a 'void *' variable, but a 'struct task_struct *'
is given. 'task_scs(tsk)' is the starting address of the task's shadow
call stack, and '__scs_magic(task_scs(tsk))' is the end address of the
task's shadow call stack.
Here should be '__scs_magic(task_scs(tsk))'.

Fixes: 5bbaf9d1fcb9 ("scs: Add support for stack usage debugging")
Signed-off-by: Jiyuan Xie <xiejiyuan@vivo.com>
Signed-off-by: Zhichi Lin <zhichi.lin@vivo.com>
---
 kernel/scs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/scs.c b/kernel/scs.c
index d7809affe..772488afd 100644
--- a/kernel/scs.c
+++ b/kernel/scs.c
@@ -135,7 +135,7 @@ static void scs_check_usage(struct task_struct *tsk)
 	if (!IS_ENABLED(CONFIG_DEBUG_STACK_USAGE))
 		return;
 
-	for (p = task_scs(tsk); p < __scs_magic(tsk); ++p) {
+	for (p = task_scs(tsk); p < __scs_magic(task_scs(tsk)); ++p) {
 		if (!READ_ONCE_NOCHECK(*p))
 			break;
 		used += sizeof(*p);
-- 
2.25.1

Re: [PATCH] scs: Fix a wrong parameter in __scs_magic

[Andrew Morton]

On Sat, 11 Oct 2025 16:22:22 +0800 Zhichi Lin <zhichi.lin@vivo.com> wrote:

> __scs_magic() needs a 'void *' variable, but a 'struct task_struct *'
> is given. 'task_scs(tsk)' is the starting address of the task's shadow
> call stack, and '__scs_magic(task_scs(tsk))' is the end address of the
> task's shadow call stack.
> Here should be '__scs_magic(task_scs(tsk))'.

What are the userspace-visible runtime effects of this bug?  Please
always describe this when fixing something.

> Fixes: 5bbaf9d1fcb9 ("scs: Add support for stack usage debugging")

This might need backporting into -stable kernels, that depends on the
answer to the above question.

> --- a/kernel/scs.c
> +++ b/kernel/scs.c
> @@ -135,7 +135,7 @@ static void scs_check_usage(struct task_struct *tsk)
>  	if (!IS_ENABLED(CONFIG_DEBUG_STACK_USAGE))
>  		return;
>  
> -	for (p = task_scs(tsk); p < __scs_magic(tsk); ++p) {
> +	for (p = task_scs(tsk); p < __scs_magic(task_scs(tsk)); ++p) {
>  		if (!READ_ONCE_NOCHECK(*p))
>  			break;
>  		used += sizeof(*p);

Thanks, I'll grab the patch for now, maybe Will would prefer to take it?

Re: [PATCH] scs: Fix a wrong parameter in __scs_magic

[林芝驰]

On 10/12/2025 1:11 AM, Andrew Morton wrote:
> On Sat, 11 Oct 2025 16:22:22 +0800 Zhichi Lin <zhichi.lin@vivo.com> wrote:
> 
>> __scs_magic() needs a 'void *' variable, but a 'struct task_struct *'
>> is given. 'task_scs(tsk)' is the starting address of the task's shadow
>> call stack, and '__scs_magic(task_scs(tsk))' is the end address of the
>> task's shadow call stack.
>> Here should be '__scs_magic(task_scs(tsk))'.
> 
> What are the userspace-visible runtime effects of this bug?  Please
> always describe this when fixing something.
> 

The user-visible effect of this bug is that when 
CONFIG_DEBUG_STACK_USAGE is enabled, the shadow call stack usage 
checking function (scs_check_usage) would scan an incorrect memory 
range. This could lead to:

1. **Inaccurate stack usage reporting**: The function would calculate 
wrong usage statistics for the shadow call stack, potentially showing 
incorrect value in kmsg.

2. **Potential kernel crash**: If the value of __scs_magic(tsk)is 
greater than that of __scs_magic(task_scs(tsk)), the for loop may access 
unmapped memory, potentially causing a kernel panic. However, this 
scenario is unlikely because task_struct is allocated via the slab 
allocator (which typically returns lower addresses), while the shadow 
call stack returned by task_scs(tsk) is allocated via vmalloc(which 
typically returns higher addresses).

However, since this is purely a debugging feature 
(CONFIG_DEBUG_STACK_USAGE), normal production systems should be not 
unaffected. The bug only impacts developers and testers who are
actively debugging stack usage with this configuration enabled.

Additionally, this issue was discovered during code reading - our 
project does not actually use this feature. For a more accurate 
assessment of real-world impact and backporting necessity, I think Will 
and Sami would be better positioned to evaluate, as they have deeper 
knowledge of how this debugging feature is actually used.

>> Fixes: 5bbaf9d1fcb9 ("scs: Add support for stack usage debugging")
> 
> This might need backporting into -stable kernels, that depends on the
> answer to the above question.
> 
>> --- a/kernel/scs.c
>> +++ b/kernel/scs.c
>> @@ -135,7 +135,7 @@ static void scs_check_usage(struct task_struct *tsk)
>>   	if (!IS_ENABLED(CONFIG_DEBUG_STACK_USAGE))
>>   		return;
>>   
>> -	for (p = task_scs(tsk); p < __scs_magic(tsk); ++p) {
>> +	for (p = task_scs(tsk); p < __scs_magic(task_scs(tsk)); ++p) {
>>   		if (!READ_ONCE_NOCHECK(*p))
>>   			break;
>>   		used += sizeof(*p);
> 
> Thanks, I'll grab the patch for now, maybe Will would prefer to take it?

Re: [PATCH] scs: Fix a wrong parameter in __scs_magic

[Will Deacon]

On Sat, Oct 11, 2025 at 10:11:17AM -0700, Andrew Morton wrote:
> On Sat, 11 Oct 2025 16:22:22 +0800 Zhichi Lin <zhichi.lin@vivo.com> wrote:
> 
> > __scs_magic() needs a 'void *' variable, but a 'struct task_struct *'
> > is given. 'task_scs(tsk)' is the starting address of the task's shadow
> > call stack, and '__scs_magic(task_scs(tsk))' is the end address of the
> > task's shadow call stack.
> > Here should be '__scs_magic(task_scs(tsk))'.
> 
> What are the userspace-visible runtime effects of this bug?  Please
> always describe this when fixing something.
> 
> > Fixes: 5bbaf9d1fcb9 ("scs: Add support for stack usage debugging")
> 
> This might need backporting into -stable kernels, that depends on the
> answer to the above question.
> 
> > --- a/kernel/scs.c
> > +++ b/kernel/scs.c
> > @@ -135,7 +135,7 @@ static void scs_check_usage(struct task_struct *tsk)
> >  	if (!IS_ENABLED(CONFIG_DEBUG_STACK_USAGE))
> >  		return;
> >  
> > -	for (p = task_scs(tsk); p < __scs_magic(tsk); ++p) {
> > +	for (p = task_scs(tsk); p < __scs_magic(task_scs(tsk)); ++p) {
> >  		if (!READ_ONCE_NOCHECK(*p))
> >  			break;
> >  		used += sizeof(*p);
> 
> Thanks, I'll grab the patch for now, maybe Will would prefer to take it?

Apologies for the slow reply, I was away for a couple of weeks.

It looks like you've already put this in -next (thank you!), so please
go ahead and send it to Linus as a fix.

Acked-by: Will Deacon <will@kernel.org>

Cheers,

Will

Re: [PATCH] scs: Fix a wrong parameter in __scs_magic

[Sami Tolvanen]

On Sat, Oct 11, 2025 at 1:23 AM Zhichi Lin <zhichi.lin@vivo.com> wrote:
>
> __scs_magic() needs a 'void *' variable, but a 'struct task_struct *'
> is given. 'task_scs(tsk)' is the starting address of the task's shadow
> call stack, and '__scs_magic(task_scs(tsk))' is the end address of the
> task's shadow call stack.
> Here should be '__scs_magic(task_scs(tsk))'.
>
> Fixes: 5bbaf9d1fcb9 ("scs: Add support for stack usage debugging")
> Signed-off-by: Jiyuan Xie <xiejiyuan@vivo.com>
> Signed-off-by: Zhichi Lin <zhichi.lin@vivo.com>
> ---
>  kernel/scs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/scs.c b/kernel/scs.c
> index d7809affe..772488afd 100644
> --- a/kernel/scs.c
> +++ b/kernel/scs.c
> @@ -135,7 +135,7 @@ static void scs_check_usage(struct task_struct *tsk)
>         if (!IS_ENABLED(CONFIG_DEBUG_STACK_USAGE))
>                 return;
>
> -       for (p = task_scs(tsk); p < __scs_magic(tsk); ++p) {
> +       for (p = task_scs(tsk); p < __scs_magic(task_scs(tsk)); ++p) {
>                 if (!READ_ONCE_NOCHECK(*p))
>                         break;
>                 used += sizeof(*p);

Looks correct to me. Thanks for the fix!

Reviewed-by: Sami Tolvanen <samitolvanen@google.com>

Sami

[PATCH] scs: Fix a wrong parameter in __scs_magic

[Wan Jiabing]

__scs_magic() needs a 'void *' variable, but a 'struct task_struct *'
is given. 'task_scs(tsk)' is the starting address of the task's shadow
call stack, and '__scs_magic(task_scs(tsk))' is the end address of the
task's shadow call stack.
Here should be '__scs_magic(task_scs(tsk))'.

Fixes: 5bbaf9d1fcb9 ("scs: Add support for stack usage debugging")
Signed-off-by: Xie Jiyuan <xiejiyuan@vivo.com>
Signed-off-by: Wan Jiabing <wanjiabing@vivo.com>
---
 kernel/scs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/scs.c b/kernel/scs.c
index b7e1b096d906..4c1ce6a6172d 100644
--- a/kernel/scs.c
+++ b/kernel/scs.c
@@ -125,7 +125,7 @@ static void scs_check_usage(struct task_struct *tsk)
 	if (!IS_ENABLED(CONFIG_DEBUG_STACK_USAGE))
 		return;
 
-	for (p = task_scs(tsk); p < __scs_magic(tsk); ++p) {
+	for (p = task_scs(tsk); p < __scs_magic(task_scs(tsk)); ++p) {
 		if (!READ_ONCE_NOCHECK(*p))
 			break;
 		used += sizeof(*p);
-- 
2.37.0

Re: [PATCH] scs: Fix a wrong parameter in __scs_magic

[Will Deacon]

On Mon, Jul 04, 2022 at 03:04:03PM +0800, Wan Jiabing wrote:
> __scs_magic() needs a 'void *' variable, but a 'struct task_struct *'
> is given. 'task_scs(tsk)' is the starting address of the task's shadow
> call stack, and '__scs_magic(task_scs(tsk))' is the end address of the
> task's shadow call stack.
> Here should be '__scs_magic(task_scs(tsk))'.
> 
> Fixes: 5bbaf9d1fcb9 ("scs: Add support for stack usage debugging")
> Signed-off-by: Xie Jiyuan <xiejiyuan@vivo.com>
> Signed-off-by: Wan Jiabing <wanjiabing@vivo.com>
> ---
>  kernel/scs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/scs.c b/kernel/scs.c
> index b7e1b096d906..4c1ce6a6172d 100644
> --- a/kernel/scs.c
> +++ b/kernel/scs.c
> @@ -125,7 +125,7 @@ static void scs_check_usage(struct task_struct *tsk)
>  	if (!IS_ENABLED(CONFIG_DEBUG_STACK_USAGE))
>  		return;
>  
> -	for (p = task_scs(tsk); p < __scs_magic(tsk); ++p) {
> +	for (p = task_scs(tsk); p < __scs_magic(task_scs(tsk)); ++p) {
>  		if (!READ_ONCE_NOCHECK(*p))
>  			break;
>  		used += sizeof(*p);

Acked-by: Will Deacon <will@kernel.org>

I'm assuming this will go via Andrew or Kees.

Will