[1/2] lib/stackdepot: Bail out in __stack_depot_save() if the stack_table is not allocated

Message ID 20230810074704.2042664-2-xiaolei.wang@windriver.com
State New
Headers
Series Bail out in __stack_depot_save() if the stack_table is not allocated and delete the kmemleak_initialized judgment in set_track_prepare() |

Commit Message

xiaolei wang Aug. 10, 2023, 7:47 a.m. UTC
  The __stack_depot_save() may be used by some subsystems even before
the stack depot is initialized. So add a check of stack_table in
__stack_depot_save() to make sure no oops in this case.

Fixes: 56a61617dd22 ("mm: use stack_depot for recording kmemleak's backtrace")
Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
---
 lib/stackdepot.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Vlastimil Babka Aug. 10, 2023, 9:53 a.m. UTC | #1
On 8/10/23 09:47, Xiaolei Wang wrote:
> The __stack_depot_save() may be used by some subsystems even before
> the stack depot is initialized.

Does that currently happen, or only after patch 2/2 it starts happening via
kmemleak?

> So add a check of stack_table in
> __stack_depot_save() to make sure no oops in this case.
> 
> Fixes: 56a61617dd22 ("mm: use stack_depot for recording kmemleak's backtrace")

In case it's only after 2/2 I don't think this is truly "Fixes"?

> Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
> ---
>  lib/stackdepot.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/stackdepot.c b/lib/stackdepot.c
> index 2f5aa851834e..a0651d013a0d 100644
> --- a/lib/stackdepot.c
> +++ b/lib/stackdepot.c
> @@ -376,7 +376,7 @@ depot_stack_handle_t __stack_depot_save(unsigned long *entries,
>  	 */
>  	nr_entries = filter_irq_stacks(entries, nr_entries);
>  
> -	if (unlikely(nr_entries == 0) || stack_depot_disabled)
> +	if (unlikely(nr_entries == 0) || stack_depot_disabled || unlikely(!stack_table))
>  		goto fast_exit;
>  
>  	hash = hash_stack(entries, nr_entries);
  
xiaolei wang Aug. 11, 2023, 2:02 a.m. UTC | #2
On 8/10/23 8:53 PM, Vlastimil Babka wrote:
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
> On 8/10/23 09:47, Xiaolei Wang wrote:
>> The __stack_depot_save() may be used by some subsystems even before
>> the stack depot is initialized.
> Does that currently happen, or only after patch 2/2 it starts happening via
> kmemleak?
Yes, currently it happens after patch 2/2 it starts happening via kmemleak,

The reason why I take it as the first patch is because I think this can 
avoid

exceptions when there is only patch2, such as when we use git bisect to 
debug

>
>> So add a check of stack_table in
>> __stack_depot_save() to make sure no oops in this case.
>>
>> Fixes: 56a61617dd22 ("mm: use stack_depot for recording kmemleak's backtrace")
> In case it's only after 2/2 I don't think this is truly "Fixes"?

Yes, it is indeed for patch2 at present, it is to fix the situation that 
kmemleak

has no backtrace in the boot phase, and at the same time, it can also 
prevent

patch2 from entering the stable kernel and causing panic

thanks

xiaolei

>
>> Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
>> ---
>>   lib/stackdepot.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/stackdepot.c b/lib/stackdepot.c
>> index 2f5aa851834e..a0651d013a0d 100644
>> --- a/lib/stackdepot.c
>> +++ b/lib/stackdepot.c
>> @@ -376,7 +376,7 @@ depot_stack_handle_t __stack_depot_save(unsigned long *entries,
>>         */
>>        nr_entries = filter_irq_stacks(entries, nr_entries);
>>
>> -     if (unlikely(nr_entries == 0) || stack_depot_disabled)
>> +     if (unlikely(nr_entries == 0) || stack_depot_disabled || unlikely(!stack_table))
>>                goto fast_exit;
>>
>>        hash = hash_stack(entries, nr_entries);
  

Patch

diff --git a/lib/stackdepot.c b/lib/stackdepot.c
index 2f5aa851834e..a0651d013a0d 100644
--- a/lib/stackdepot.c
+++ b/lib/stackdepot.c
@@ -376,7 +376,7 @@  depot_stack_handle_t __stack_depot_save(unsigned long *entries,
 	 */
 	nr_entries = filter_irq_stacks(entries, nr_entries);
 
-	if (unlikely(nr_entries == 0) || stack_depot_disabled)
+	if (unlikely(nr_entries == 0) || stack_depot_disabled || unlikely(!stack_table))
 		goto fast_exit;
 
 	hash = hash_stack(entries, nr_entries);