| Message ID | 20190410103645.862294081@linutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
On Wed, Apr 10, 2019 at 1:06 PM Thomas Gleixner <tglx@linutronix.de> wrote: > > Replace the indirection through struct stack_trace by using the storage > array based interfaces. > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > Cc: Andrey Ryabinin <aryabinin@virtuozzo.com> > Cc: Alexander Potapenko <glider@google.com> > Cc: Dmitry Vyukov <dvyukov@google.com> > Cc: kasan-dev@googlegroups.com > Cc: linux-mm@kvack.org > --- > mm/kasan/common.c | 30 ++++++++++++------------------ > mm/kasan/report.c | 7 ++++--- > 2 files changed, 16 insertions(+), 21 deletions(-) > > --- a/mm/kasan/common.c > +++ b/mm/kasan/common.c > @@ -48,34 +48,28 @@ static inline int in_irqentry_text(unsig > ptr < (unsigned long)&__softirqentry_text_end); > } > > -static inline void filter_irq_stacks(struct stack_trace *trace) > +static inline unsigned int filter_irq_stacks(unsigned long *entries, > + unsigned int nr_entries) > { > - int i; > + unsigned int i; > > - if (!trace->nr_entries) > - return; > - for (i = 0; i < trace->nr_entries; i++) > - if (in_irqentry_text(trace->entries[i])) { > + for (i = 0; i < nr_entries; i++) { > + if (in_irqentry_text(entries[i])) { > /* Include the irqentry function into the stack. */ > - trace->nr_entries = i + 1; > - break; > + return i + 1; > } > + } > + return nr_entries; > } > > static inline depot_stack_handle_t save_stack(gfp_t flags) > { > unsigned long entries[KASAN_STACK_DEPTH]; > - struct stack_trace trace = { > - .nr_entries = 0, > - .entries = entries, > - .max_entries = KASAN_STACK_DEPTH, > - .skip = 0 > - }; > + unsigned int nent; > > - save_stack_trace(&trace); > - filter_irq_stacks(&trace); > - > - return depot_save_stack(&trace, flags); > + nent = stack_trace_save(entries, ARRAY_SIZE(entries), 0); > + nent = filter_irq_stacks(entries, nent); > + return stack_depot_save(entries, nent, flags); > } > > static inline void set_track(struct kasan_track *track, gfp_t flags) > --- a/mm/kasan/report.c > +++ b/mm/kasan/report.c > @@ -100,10 +100,11 @@ static void print_track(struct kasan_tra > { > pr_err("%s by task %u:\n", prefix, track->pid); > if (track->stack) { > - struct stack_trace trace; > + unsigned long *entries; > + unsigned int nent; > > - depot_fetch_stack(track->stack, &trace); > - print_stack_trace(&trace, 0); > + nent = stack_depot_fetch(track->stack, &entries); > + stack_trace_print(entries, nent, 0); > } else { > pr_err("(stack is not available)\n"); > } Acked-by: Dmitry Vyukov <dvyukov@google.com>
On Wed, Apr 10, 2019 at 12:28:19PM +0200, Thomas Gleixner wrote: > Replace the indirection through struct stack_trace by using the storage > array based interfaces. > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > Cc: Andrey Ryabinin <aryabinin@virtuozzo.com> > Cc: Alexander Potapenko <glider@google.com> > Cc: Dmitry Vyukov <dvyukov@google.com> > Cc: kasan-dev@googlegroups.com > Cc: linux-mm@kvack.org > --- > mm/kasan/common.c | 30 ++++++++++++------------------ > mm/kasan/report.c | 7 ++++--- > 2 files changed, 16 insertions(+), 21 deletions(-) > > --- a/mm/kasan/common.c > +++ b/mm/kasan/common.c > @@ -48,34 +48,28 @@ static inline int in_irqentry_text(unsig > ptr < (unsigned long)&__softirqentry_text_end); > } > > -static inline void filter_irq_stacks(struct stack_trace *trace) > +static inline unsigned int filter_irq_stacks(unsigned long *entries, > + unsigned int nr_entries) > { > - int i; > + unsigned int i; > > - if (!trace->nr_entries) > - return; > - for (i = 0; i < trace->nr_entries; i++) > - if (in_irqentry_text(trace->entries[i])) { > + for (i = 0; i < nr_entries; i++) { > + if (in_irqentry_text(entries[i])) { > /* Include the irqentry function into the stack. */ > - trace->nr_entries = i + 1; > - break; > + return i + 1; Isn't this an off-by-one error if "i" points to the last entry of the array?
On Wed, 10 Apr 2019, Josh Poimboeuf wrote: > On Wed, Apr 10, 2019 at 12:28:19PM +0200, Thomas Gleixner wrote: > > Replace the indirection through struct stack_trace by using the storage > > array based interfaces. > > > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > > Cc: Andrey Ryabinin <aryabinin@virtuozzo.com> > > Cc: Alexander Potapenko <glider@google.com> > > Cc: Dmitry Vyukov <dvyukov@google.com> > > Cc: kasan-dev@googlegroups.com > > Cc: linux-mm@kvack.org > > --- > > mm/kasan/common.c | 30 ++++++++++++------------------ > > mm/kasan/report.c | 7 ++++--- > > 2 files changed, 16 insertions(+), 21 deletions(-) > > > > --- a/mm/kasan/common.c > > +++ b/mm/kasan/common.c > > @@ -48,34 +48,28 @@ static inline int in_irqentry_text(unsig > > ptr < (unsigned long)&__softirqentry_text_end); > > } > > > > -static inline void filter_irq_stacks(struct stack_trace *trace) > > +static inline unsigned int filter_irq_stacks(unsigned long *entries, > > + unsigned int nr_entries) > > { > > - int i; > > + unsigned int i; > > > > - if (!trace->nr_entries) > > - return; > > - for (i = 0; i < trace->nr_entries; i++) > > - if (in_irqentry_text(trace->entries[i])) { > > + for (i = 0; i < nr_entries; i++) { > > + if (in_irqentry_text(entries[i])) { > > /* Include the irqentry function into the stack. */ > > - trace->nr_entries = i + 1; > > - break; > > + return i + 1; > > Isn't this an off-by-one error if "i" points to the last entry of the > array? Yes, copied one ...
On Sun, 14 Apr 2019, Thomas Gleixner wrote: > On Wed, 10 Apr 2019, Josh Poimboeuf wrote: > > On Wed, Apr 10, 2019 at 12:28:19PM +0200, Thomas Gleixner wrote: > > > Replace the indirection through struct stack_trace by using the storage > > > array based interfaces. > > > > > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > > > Cc: Andrey Ryabinin <aryabinin@virtuozzo.com> > > > Cc: Alexander Potapenko <glider@google.com> > > > Cc: Dmitry Vyukov <dvyukov@google.com> > > > Cc: kasan-dev@googlegroups.com > > > Cc: linux-mm@kvack.org > > > --- > > > mm/kasan/common.c | 30 ++++++++++++------------------ > > > mm/kasan/report.c | 7 ++++--- > > > 2 files changed, 16 insertions(+), 21 deletions(-) > > > > > > --- a/mm/kasan/common.c > > > +++ b/mm/kasan/common.c > > > @@ -48,34 +48,28 @@ static inline int in_irqentry_text(unsig > > > ptr < (unsigned long)&__softirqentry_text_end); > > > } > > > > > > -static inline void filter_irq_stacks(struct stack_trace *trace) > > > +static inline unsigned int filter_irq_stacks(unsigned long *entries, > > > + unsigned int nr_entries) > > > { > > > - int i; > > > + unsigned int i; > > > > > > - if (!trace->nr_entries) > > > - return; > > > - for (i = 0; i < trace->nr_entries; i++) > > > - if (in_irqentry_text(trace->entries[i])) { > > > + for (i = 0; i < nr_entries; i++) { > > > + if (in_irqentry_text(entries[i])) { > > > /* Include the irqentry function into the stack. */ > > > - trace->nr_entries = i + 1; > > > - break; > > > + return i + 1; > > > > Isn't this an off-by-one error if "i" points to the last entry of the > > array? > > Yes, copied one ... Oh, no. The point is that it returns the number of stack entries to store. So if i == nr_entries - 1, then it returns nr_entries, i.e. all entries are stored. Thanks, tglx
--- a/mm/kasan/common.c +++ b/mm/kasan/common.c @@ -48,34 +48,28 @@ static inline int in_irqentry_text(unsig ptr < (unsigned long)&__softirqentry_text_end); } -static inline void filter_irq_stacks(struct stack_trace *trace) +static inline unsigned int filter_irq_stacks(unsigned long *entries, + unsigned int nr_entries) { - int i; + unsigned int i; - if (!trace->nr_entries) - return; - for (i = 0; i < trace->nr_entries; i++) - if (in_irqentry_text(trace->entries[i])) { + for (i = 0; i < nr_entries; i++) { + if (in_irqentry_text(entries[i])) { /* Include the irqentry function into the stack. */ - trace->nr_entries = i + 1; - break; + return i + 1; } + } + return nr_entries; } static inline depot_stack_handle_t save_stack(gfp_t flags) { unsigned long entries[KASAN_STACK_DEPTH]; - struct stack_trace trace = { - .nr_entries = 0, - .entries = entries, - .max_entries = KASAN_STACK_DEPTH, - .skip = 0 - }; + unsigned int nent; - save_stack_trace(&trace); - filter_irq_stacks(&trace); - - return depot_save_stack(&trace, flags); + nent = stack_trace_save(entries, ARRAY_SIZE(entries), 0); + nent = filter_irq_stacks(entries, nent); + return stack_depot_save(entries, nent, flags); } static inline void set_track(struct kasan_track *track, gfp_t flags) --- a/mm/kasan/report.c +++ b/mm/kasan/report.c @@ -100,10 +100,11 @@ static void print_track(struct kasan_tra { pr_err("%s by task %u:\n", prefix, track->pid); if (track->stack) { - struct stack_trace trace; + unsigned long *entries; + unsigned int nent; - depot_fetch_stack(track->stack, &trace); - print_stack_trace(&trace, 0); + nent = stack_depot_fetch(track->stack, &entries); + stack_trace_print(entries, nent, 0); } else { pr_err("(stack is not available)\n"); }
Replace the indirection through struct stack_trace by using the storage array based interfaces. Signed-off-by: Thomas Gleixner <tglx@linutronix.de> Cc: Andrey Ryabinin <aryabinin@virtuozzo.com> Cc: Alexander Potapenko <glider@google.com> Cc: Dmitry Vyukov <dvyukov@google.com> Cc: kasan-dev@googlegroups.com Cc: linux-mm@kvack.org --- mm/kasan/common.c | 30 ++++++++++++------------------ mm/kasan/report.c | 7 ++++--- 2 files changed, 16 insertions(+), 21 deletions(-)