Re: bio linked list corruption.

2016-10-20 Thread Thomas Gleixner 
On Thu, 20 Oct 2016, Ingo Molnar wrote:
> * Linus Torvalds  wrote:
> > So I don't think it's related. Yours looks like some subtle timer base
> > race. It smells like a locking problem with timers. I'm not seeing
> > what it might be, but it *might* have been fixed by doing the
> > TIMER_MIGRATING bit right in add_timer_on() (commit 22b886dd1018).

4.1 does not have the new fangled timer wheel. So 22b886dd1018 is
irrelevant.

> > Adding some timer people just in case, but I don't think your 4.1
> > report is related.

This looks like one of the classic issues where an enqueued timer gets
freed/reinitialized or otherwise manipulated. So yes, Ingos recommendation
for debugging is the right thing to do.
 
Thanks,

tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/3] vsprintf: Remove accidental VLA usage

2018-03-08 Thread Thomas Gleixner 
On Thu, 8 Mar 2018, Rasmus Villemoes wrote:
> On 2018-03-08 04:30, Kees Cook wrote:
> >  /**
> > + * SIMPLE_MAX - return maximum of two values without any type checking
> > + * @x: first value
> > + * @y: second value
> > + *
> > + * This should only be used in stack array sizes, since the type-checking
> > + * from max() confuses the compiler into thinking a VLA is being used.
> > + */
> > +#define SIMPLE_MAX(x, y)   ((size_t)(x) > (size_t)(y) ? (size_t)(x) \
> > +  : (size_t)(y))
> 
> This will be abused at some point, leading to the usual double
> evaluation etc. etc. problems. The name is also too long (and in general
> we should avoid adjectives like "simple", "safe", people reading the
> code won't know what is simple or safe about it). I think this should work
> 
> #define MAX(x, y) (__builtin_choose_expr((x) > (y), x, y))
> 
> That forces (x)>(y) to be a compile-time constant, so x and y must also
> be; hence there can be no side effects. The MIN version of this could
> replace the custom __const_min in fs/file.c, and probably other places
> as well.
> 
> I tested that this at least works in the vsprintf case, -Wvla no longer
> complains. fs/file.c also compiles with the MIN version of this.
> 
> I suppose MIN and MAX will collide with other uses in the tree. Hmm.

Make it CONST_MAX() or something like that which makes it entirely clear.

Thanks,

tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch V2 09/29] mm/kasan: Simplify stacktrace handling

2019-04-18 Thread Thomas Gleixner 
On Thu, 18 Apr 2019, Andrey Ryabinin wrote:
> On 4/18/19 11:41 AM, Thomas Gleixner wrote:
> > Replace the indirection through struct stack_trace by using the storage
> > array based interfaces.
> > 
> > Signed-off-by: Thomas Gleixner 
> > Acked-by: Dmitry Vyukov 
> > Cc: Andrey Ryabinin 
> > Cc: Alexander Potapenko 
> > Cc: kasan-...@googlegroups.com
> > Cc: linux...@kvack.org
> 
> Acked-by: Andrey Ryabinin 
> 
> >  
> >  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 nr_entries;
> >  
> > -   save_stack_trace(&trace);
> > -   filter_irq_stacks(&trace);
> > -
> > -   return depot_save_stack(&trace, flags);
> > +   nr_entries = stack_trace_save(entries, ARRAY_SIZE(entries), 0);
> > +   nr_entries = filter_irq_stacks(entries, nr_entries);
> > +   return stack_depot_save(entries, nr_entries, flags);
> 
> Suggestion for further improvement:
> 
> stack_trace_save() shouldn't unwind beyond irq entry point so we wouldn't
> need filter_irq_stacks().  Probably all call sites doesn't care about
> random stack above irq entry point, so it doesn't make sense to spend
> resources on unwinding non-irq stack from interrupt first an filtering
> out it later.

There are users which care about the full trace.

Once we have cleaned up the whole architeture side, we can add core side
filtering which allows to

   1) replace the 'skip number of entries at the beginning

   2) stop the trace when it reaches a certain point

Right now, I don't want to change any of this until the whole mess is
consolidated.

Thanks,

tglx