On Fri, 2020-09-25 at 10:55 +0200, Thomas Gleixner wrote: > Walter, > > On Fri, Sep 25 2020 at 15:18, Walter Wu wrote: > > On Thu, 2020-09-24 at 23:41 +0200, Thomas Gleixner wrote: > >> > For timers it has turned out to be useful to record the stack trace > >> > of the timer init call. > >> > >> In which way? And what kind of bug does it catch which cannot be catched > >> by existing debug mechanisms already? > >> > > We only provide another debug mechanisms to debug use-after-free or > > double-free, it can be displayed together in KASAN report and have a > > chance to debug, and it doesn't need to enable existing debug mechanisms > > at the same time. then it has a chance to resolve issue. > > Again. KASAN can only cover UAF, but there are a dozen other ways to > wreck the system with wrong usage of timers which can't be caught by > KASAN. > > >> > Because if the UAF root cause is in timer init, then user can see > >> > KASAN report to get where it is registered and find out the root > >> > cause. > >> > >> What? If the UAF root cause is in timer init, then registering it after > >> using it in that very same function is pretty pointless. > >> > > See [1], the call stack shows UAF happen at dummy_timer(), it is the > > callback function and set by timer_setup(), if KASAN report shows the > > timer call stack, it should be useful for programmer. > > The report you linked to has absolutely nothing to do with a timer > related UAF. The timer callback calls kfree_skb() on something which is > already freed. So the root cause of this is NOT in timer init as you > claimed above. The timer callback is just exposing a problem in the URB > management of this driver. IOW the recording of the timer init stack is > completely useless for decoding this problem. > > >> There is a lot of handwaving how useful this is, but TBH I don't see the > >> value at all. > >> > >> DEBUG_OBJECTS_TIMERS does a lot more than crashing on UAF. If KASAN > >> provides additional value over DEBUG_OBJECTS_TIMERS then spell it out, > >> but just saying that you don't need to enable DEBUG_OBJECTS_TIMERS is > >> not making an argument for that change. > >> > > We don't want to replace DEBUG_OBJECTS_TIMERS with this patches, only > > hope to use low overhead(compare with DEBUG_OBJECTS_TIMERS) to debug > > KASAN has lower overhead than DEBUG_OBJECTS_TIMERS? Maybe in a different > universe. > I mean KASAN + our patch vs KASAN + DEBUG_OBJECTS_TIMERS. The front one have the information to the original caller and help to debug. It is smaller overhead than the one behind. > That said, I'm not opposed to the change per se, but without a sensible > justification this is just pointless. > > Sprinkling kasan_foo() all over the place and claiming it's useful > without a valid example does not provide any value. > > Quite the contrary it gives the completely wrong sense what KASAN can do > and what not. > I agree your saying, so that I need to find out a use case to explain to you. Thanks Walter > Thanks, > > tglx >
