* [RFC PATCH v2 0/2] lib, stackdepot: check stackdepot handle before accessing slabs @ 2021-09-02 0:01 Imran Khan 2021-09-02 0:01 ` [RFC PATCH v2 1/2] " Imran Khan 2021-09-02 0:01 ` [RFC PATCH v2 2/2] lib, stackdepot: Add helper to print stack entries Imran Khan 0 siblings, 2 replies; 8+ messages in thread From: Imran Khan @ 2021-09-02 0:01 UTC (permalink / raw) To: vbabka, geert, akpm, ryabinin.a.a, glider, andreyknvl, dvyukov Cc: linux-mm, linux-kernel Original cover letter --------------------------------------- This RFC patch series addresses suggestion discussed in an earlier RFC [1]. Since earlier RFC was about SLUB subsystem, and current changes only involve stackdepot, I am submitting the patches in a new thread. The changes of this patch set are as follows: PATCH-1: Checks validity of a stackdepot handle before proceeding to access stackdepot slab/objects. PATCH-2: Adds a helper in stackdepot, to allow users to print stack entries just by specifying the stackdepot handle. [1] https://lore.kernel.org/lkml/2772cf56-4183-857f-d070-c54bceb5c8d9@suse.cz/ -------------------------------------- Changes in v2: - remove WARN messages - add declaration of stack_depot_print in stackdepot.h - make current users, which use stack_depot_fetch + stack_trace_print to print stack entries, use stack_depot_print Imran Khan (2): lib, stackdepot: check stackdepot handle before accessing slabs. lib, stackdepot: Add helper to print stack entries. include/linux/stackdepot.h | 2 ++ lib/stackdepot.c | 20 ++++++++++++++++++++ mm/kasan/report.c | 15 +++------------ mm/page_owner.c | 13 ++++--------- 4 files changed, 29 insertions(+), 21 deletions(-) -- 2.30.2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [RFC PATCH v2 1/2] lib, stackdepot: check stackdepot handle before accessing slabs. 2021-09-02 0:01 [RFC PATCH v2 0/2] lib, stackdepot: check stackdepot handle before accessing slabs Imran Khan @ 2021-09-02 0:01 ` Imran Khan 2021-09-02 7:53 ` Vlastimil Babka 2021-09-02 0:01 ` [RFC PATCH v2 2/2] lib, stackdepot: Add helper to print stack entries Imran Khan 1 sibling, 1 reply; 8+ messages in thread From: Imran Khan @ 2021-09-02 0:01 UTC (permalink / raw) To: vbabka, geert, akpm, ryabinin.a.a, glider, andreyknvl, dvyukov Cc: linux-mm, linux-kernel stack_depot_save allocates slabs that will be used for storing objects in future.If this slab allocation fails we may get to a situation where space allocation for a new stack_record fails, causing stack_depot_save to return 0 as handle. If user of this handle ends up invoking stack_depot_fetch with this handle value, current implementation of stack_depot_fetch will end up using slab from wrong index. To avoid this check handle value at the beginning. Signed-off-by: Imran Khan <imran.f.khan@oracle.com> Suggested-by: Vlastimil Babka <vbabka@suse.cz> --- lib/stackdepot.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/stackdepot.c b/lib/stackdepot.c index 0a2e417f83cb..67439c082490 100644 --- a/lib/stackdepot.c +++ b/lib/stackdepot.c @@ -232,6 +232,9 @@ unsigned int stack_depot_fetch(depot_stack_handle_t handle, struct stack_record *stack; *entries = NULL; + if (!handle) + return 0; + if (parts.slabindex > depot_index) { WARN(1, "slab index %d out of bounds (%d) for stack id %08x\n", parts.slabindex, depot_index, handle); -- 2.30.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [RFC PATCH v2 1/2] lib, stackdepot: check stackdepot handle before accessing slabs. 2021-09-02 0:01 ` [RFC PATCH v2 1/2] " Imran Khan @ 2021-09-02 7:53 ` Vlastimil Babka 0 siblings, 0 replies; 8+ messages in thread From: Vlastimil Babka @ 2021-09-02 7:53 UTC (permalink / raw) To: Imran Khan, geert, akpm, ryabinin.a.a, glider, andreyknvl, dvyukov Cc: linux-mm, linux-kernel On 9/2/21 02:01, Imran Khan wrote: > stack_depot_save allocates slabs that will be used for storing > objects in future.If this slab allocation fails we may get to > a situation where space allocation for a new stack_record fails, > causing stack_depot_save to return 0 as handle. > If user of this handle ends up invoking stack_depot_fetch with > this handle value, current implementation of stack_depot_fetch > will end up using slab from wrong index. > To avoid this check handle value at the beginning. > > Signed-off-by: Imran Khan <imran.f.khan@oracle.com> > Suggested-by: Vlastimil Babka <vbabka@suse.cz> Acked-by: Vlastimil Babka <vbabka@suse.cz> > --- > lib/stackdepot.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/lib/stackdepot.c b/lib/stackdepot.c > index 0a2e417f83cb..67439c082490 100644 > --- a/lib/stackdepot.c > +++ b/lib/stackdepot.c > @@ -232,6 +232,9 @@ unsigned int stack_depot_fetch(depot_stack_handle_t handle, > struct stack_record *stack; > > *entries = NULL; > + if (!handle) > + return 0; > + > if (parts.slabindex > depot_index) { > WARN(1, "slab index %d out of bounds (%d) for stack id %08x\n", > parts.slabindex, depot_index, handle); > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [RFC PATCH v2 2/2] lib, stackdepot: Add helper to print stack entries. 2021-09-02 0:01 [RFC PATCH v2 0/2] lib, stackdepot: check stackdepot handle before accessing slabs Imran Khan 2021-09-02 0:01 ` [RFC PATCH v2 1/2] " Imran Khan @ 2021-09-02 0:01 ` Imran Khan 2021-09-02 7:55 ` Vlastimil Babka 2021-09-14 9:01 ` Vlastimil Babka 1 sibling, 2 replies; 8+ messages in thread From: Imran Khan @ 2021-09-02 0:01 UTC (permalink / raw) To: vbabka, geert, akpm, ryabinin.a.a, glider, andreyknvl, dvyukov Cc: linux-mm, linux-kernel To print a stack entries, users of stackdepot, first use stack_depot_fetch to get a list of stack entries and then use stack_trace_print to print this list. Provide a helper in stackdepot to print stack entries based on stackdepot handle. Also change above mentioned users to use this helper. Signed-off-by: Imran Khan <imran.f.khan@oracle.com> Suggested-by: Vlastimil Babka <vbabka@suse.cz> --- include/linux/stackdepot.h | 2 ++ lib/stackdepot.c | 17 +++++++++++++++++ mm/kasan/report.c | 15 +++------------ mm/page_owner.c | 13 ++++--------- 4 files changed, 26 insertions(+), 21 deletions(-) diff --git a/include/linux/stackdepot.h b/include/linux/stackdepot.h index 6bb4bc1a5f54..d77a30543dd4 100644 --- a/include/linux/stackdepot.h +++ b/include/linux/stackdepot.h @@ -19,6 +19,8 @@ depot_stack_handle_t stack_depot_save(unsigned long *entries, unsigned int stack_depot_fetch(depot_stack_handle_t handle, unsigned long **entries); +void stack_depot_print(depot_stack_handle_t stack); + unsigned int filter_irq_stacks(unsigned long *entries, unsigned int nr_entries); #ifdef CONFIG_STACKDEPOT diff --git a/lib/stackdepot.c b/lib/stackdepot.c index 67439c082490..873aeb152f52 100644 --- a/lib/stackdepot.c +++ b/lib/stackdepot.c @@ -214,6 +214,23 @@ static inline struct stack_record *find_stack(struct stack_record *bucket, return NULL; } +/** + * stack_depot_print - print stack entries from a depot + * + * @handle: Stack depot handle which was returned from + * stack_depot_save(). + * + */ +void stack_depot_print(depot_stack_handle_t stack) +{ + unsigned long *entries; + unsigned int nr_entries; + + nr_entries = stack_depot_fetch(stack, &entries); + stack_trace_print(entries, nr_entries, 0); +} +EXPORT_SYMBOL_GPL(stack_depot_print); + /** * stack_depot_fetch - Fetch stack entries from a depot * diff --git a/mm/kasan/report.c b/mm/kasan/report.c index 884a950c7026..3239fd8f8747 100644 --- a/mm/kasan/report.c +++ b/mm/kasan/report.c @@ -132,20 +132,11 @@ static void end_report(unsigned long *flags, unsigned long addr) kasan_enable_current(); } -static void print_stack(depot_stack_handle_t stack) -{ - unsigned long *entries; - unsigned int nr_entries; - - nr_entries = stack_depot_fetch(stack, &entries); - stack_trace_print(entries, nr_entries, 0); -} - static void print_track(struct kasan_track *track, const char *prefix) { pr_err("%s by task %u:\n", prefix, track->pid); if (track->stack) { - print_stack(track->stack); + stack_depot_print(track->stack); } else { pr_err("(stack is not available)\n"); } @@ -214,12 +205,12 @@ static void describe_object_stacks(struct kmem_cache *cache, void *object, return; if (alloc_meta->aux_stack[0]) { pr_err("Last potentially related work creation:\n"); - print_stack(alloc_meta->aux_stack[0]); + stack_depot_print(alloc_meta->aux_stack[0]); pr_err("\n"); } if (alloc_meta->aux_stack[1]) { pr_err("Second to last potentially related work creation:\n"); - print_stack(alloc_meta->aux_stack[1]); + stack_depot_print(alloc_meta->aux_stack[1]); pr_err("\n"); } #endif diff --git a/mm/page_owner.c b/mm/page_owner.c index d24ed221357c..7918770c2b2b 100644 --- a/mm/page_owner.c +++ b/mm/page_owner.c @@ -394,8 +394,6 @@ void __dump_page_owner(const struct page *page) struct page_ext *page_ext = lookup_page_ext(page); struct page_owner *page_owner; depot_stack_handle_t handle; - unsigned long *entries; - unsigned int nr_entries; gfp_t gfp_mask; int mt; @@ -423,20 +421,17 @@ void __dump_page_owner(const struct page *page) page_owner->pid, page_owner->ts_nsec, page_owner->free_ts_nsec); handle = READ_ONCE(page_owner->handle); - if (!handle) { + if (!handle) pr_alert("page_owner allocation stack trace missing\n"); - } else { - nr_entries = stack_depot_fetch(handle, &entries); - stack_trace_print(entries, nr_entries, 0); - } + else + stack_depot_print(handle); handle = READ_ONCE(page_owner->free_handle); if (!handle) { pr_alert("page_owner free stack trace missing\n"); } else { - nr_entries = stack_depot_fetch(handle, &entries); pr_alert("page last free stack trace:\n"); - stack_trace_print(entries, nr_entries, 0); + stack_depot_print(handle); } if (page_owner->last_migrate_reason != -1) -- 2.30.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [RFC PATCH v2 2/2] lib, stackdepot: Add helper to print stack entries. 2021-09-02 0:01 ` [RFC PATCH v2 2/2] lib, stackdepot: Add helper to print stack entries Imran Khan @ 2021-09-02 7:55 ` Vlastimil Babka 2021-09-03 16:03 ` Alexander Potapenko 2021-09-14 9:01 ` Vlastimil Babka 1 sibling, 1 reply; 8+ messages in thread From: Vlastimil Babka @ 2021-09-02 7:55 UTC (permalink / raw) To: Imran Khan, geert, akpm, ryabinin.a.a, glider, andreyknvl, dvyukov Cc: linux-mm, linux-kernel On 9/2/21 02:01, Imran Khan wrote: > To print a stack entries, users of stackdepot, first > use stack_depot_fetch to get a list of stack entries > and then use stack_trace_print to print this list. > Provide a helper in stackdepot to print stack entries > based on stackdepot handle. > Also change above mentioned users to use this helper. > > Signed-off-by: Imran Khan <imran.f.khan@oracle.com> > Suggested-by: Vlastimil Babka <vbabka@suse.cz> Acked-by: Vlastimil Babka <vbabka@suse.cz> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH v2 2/2] lib, stackdepot: Add helper to print stack entries. 2021-09-02 7:55 ` Vlastimil Babka @ 2021-09-03 16:03 ` Alexander Potapenko 0 siblings, 0 replies; 8+ messages in thread From: Alexander Potapenko @ 2021-09-03 16:03 UTC (permalink / raw) To: Vlastimil Babka Cc: Imran Khan, Geert Uytterhoeven, Andrew Morton, Andrey Ryabinin, Andrey Konovalov, Dmitriy Vyukov, Linux Memory Management List, LKML On Thu, Sep 2, 2021 at 9:55 AM Vlastimil Babka <vbabka@suse.cz> wrote: > > On 9/2/21 02:01, Imran Khan wrote: > > To print a stack entries, users of stackdepot, first > > use stack_depot_fetch to get a list of stack entries > > and then use stack_trace_print to print this list. > > Provide a helper in stackdepot to print stack entries > > based on stackdepot handle. > > Also change above mentioned users to use this helper. > > > > Signed-off-by: Imran Khan <imran.f.khan@oracle.com> > > Suggested-by: Vlastimil Babka <vbabka@suse.cz> > > Acked-by: Vlastimil Babka <vbabka@suse.cz> Reviewed-by: Alexander Potapenko <glider@google.com> -- Alexander Potapenko Software Engineer Google Germany GmbH Erika-Mann-Straße, 33 80636 München Geschäftsführer: Paul Manicle, Halimah DeLaine Prado Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH v2 2/2] lib, stackdepot: Add helper to print stack entries. 2021-09-02 0:01 ` [RFC PATCH v2 2/2] lib, stackdepot: Add helper to print stack entries Imran Khan 2021-09-02 7:55 ` Vlastimil Babka @ 2021-09-14 9:01 ` Vlastimil Babka 2021-09-15 3:29 ` imran.f.khan 1 sibling, 1 reply; 8+ messages in thread From: Vlastimil Babka @ 2021-09-14 9:01 UTC (permalink / raw) To: Imran Khan, geert, akpm, ryabinin.a.a, glider, andreyknvl, dvyukov Cc: linux-mm, linux-kernel On 9/2/21 02:01, Imran Khan wrote: > To print a stack entries, users of stackdepot, first > use stack_depot_fetch to get a list of stack entries > and then use stack_trace_print to print this list. > Provide a helper in stackdepot to print stack entries > based on stackdepot handle. > Also change above mentioned users to use this helper. > > Signed-off-by: Imran Khan <imran.f.khan@oracle.com> > Suggested-by: Vlastimil Babka <vbabka@suse.cz> > --- > include/linux/stackdepot.h | 2 ++ > lib/stackdepot.c | 17 +++++++++++++++++ > mm/kasan/report.c | 15 +++------------ > mm/page_owner.c | 13 ++++--------- > 4 files changed, 26 insertions(+), 21 deletions(-) > > diff --git a/include/linux/stackdepot.h b/include/linux/stackdepot.h > index 6bb4bc1a5f54..d77a30543dd4 100644 > --- a/include/linux/stackdepot.h > +++ b/include/linux/stackdepot.h > @@ -19,6 +19,8 @@ depot_stack_handle_t stack_depot_save(unsigned long *entries, > unsigned int stack_depot_fetch(depot_stack_handle_t handle, > unsigned long **entries); > > +void stack_depot_print(depot_stack_handle_t stack); > + > unsigned int filter_irq_stacks(unsigned long *entries, unsigned int nr_entries); > > #ifdef CONFIG_STACKDEPOT > diff --git a/lib/stackdepot.c b/lib/stackdepot.c > index 67439c082490..873aeb152f52 100644 > --- a/lib/stackdepot.c > +++ b/lib/stackdepot.c > @@ -214,6 +214,23 @@ static inline struct stack_record *find_stack(struct stack_record *bucket, > return NULL; > } > > +/** > + * stack_depot_print - print stack entries from a depot > + * > + * @handle: Stack depot handle which was returned from > + * stack_depot_save(). > + * > + */ > +void stack_depot_print(depot_stack_handle_t stack) > +{ > + unsigned long *entries; > + unsigned int nr_entries; > + > + nr_entries = stack_depot_fetch(stack, &entries); Maybe this should also skip stack_trace_print when nr_entries is 0, to avoid the warning. While the callers added by this patch check handle != 0, future ones might not. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH v2 2/2] lib, stackdepot: Add helper to print stack entries. 2021-09-14 9:01 ` Vlastimil Babka @ 2021-09-15 3:29 ` imran.f.khan 0 siblings, 0 replies; 8+ messages in thread From: imran.f.khan @ 2021-09-15 3:29 UTC (permalink / raw) To: Vlastimil Babka, geert, akpm, ryabinin.a.a, glider, andreyknvl, dvyukov Cc: linux-mm, linux-kernel On 14/9/21 7:01 pm, Vlastimil Babka wrote: > On 9/2/21 02:01, Imran Khan wrote: >> To print a stack entries, users of stackdepot, first >> use stack_depot_fetch to get a list of stack entries >> and then use stack_trace_print to print this list. >> Provide a helper in stackdepot to print stack entries >> based on stackdepot handle. >> Also change above mentioned users to use this helper. >> [...] >> >> +/** >> + * stack_depot_print - print stack entries from a depot >> + * >> + * @handle: Stack depot handle which was returned from >> + * stack_depot_save(). >> + * >> + */ >> +void stack_depot_print(depot_stack_handle_t stack) >> +{ >> + unsigned long *entries; >> + unsigned int nr_entries; >> + >> + nr_entries = stack_depot_fetch(stack, &entries); > > Maybe this should also skip stack_trace_print when nr_entries is 0, to avoid > the warning. While the callers added by this patch check handle != 0, future > ones might not. > Agree. I have incorporated this suggestion in latest version [1] [1] https://lore.kernel.org/lkml/20210915014806.3206938-1-imran.f.khan@oracle.com/ Thanks -- Imran ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-09-15 3:30 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-09-02 0:01 [RFC PATCH v2 0/2] lib, stackdepot: check stackdepot handle before accessing slabs Imran Khan 2021-09-02 0:01 ` [RFC PATCH v2 1/2] " Imran Khan 2021-09-02 7:53 ` Vlastimil Babka 2021-09-02 0:01 ` [RFC PATCH v2 2/2] lib, stackdepot: Add helper to print stack entries Imran Khan 2021-09-02 7:55 ` Vlastimil Babka 2021-09-03 16:03 ` Alexander Potapenko 2021-09-14 9:01 ` Vlastimil Babka 2021-09-15 3:29 ` imran.f.khan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).