linux-kernel - Re: [PATCH 1/2] lib/stackdepot: Bail out in __stack_depot_save() if the stack

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0651ed3c-7239-0235-ac14-b0379fe06297@windriver.com>
Date:   Fri, 11 Aug 2023 13:02:39 +1100
From:   wang xiaolei <xiaolei.wang@...driver.com>
To:     Vlastimil Babka <vbabka@...e.cz>, catalin.marinas@....com,
        akpm@...ux-foundation.org, glider@...gle.com, andreyknvl@...il.com,
        zhaoyang.huang@...soc.com
Cc:     linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] lib/stackdepot: Bail out in __stack_depot_save() if
 the stack_table is not allocated


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@...driver.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);