On Sun, May 29, 2016 at 4:45 PM, Luruo, Kuthonuzo
<kuthonuzo.luruo@xxxxxxx> wrote:
>> > +/* flags shadow for object header if it has been overwritten. */
>> > +void kasan_mark_bad_meta(struct kasan_alloc_meta *alloc_info,
>> > + struct kasan_access_info *info)
>> > +{
>> > + u8 *datap = (u8 *)&alloc_info->data;
>> > +
>> > + if ((((u8 *)info->access_addr + info->access_size) > datap) &&
>> > + ((u8 *)info->first_bad_addr <= datap) &&
>> > + info->is_write)
>> > + kasan_poison_shadow((void *)datap, KASAN_SHADOW_SCALE_SIZE,
>> > + KASAN_KMALLOC_BAD_META);
>>
>>
>> Is it only to prevent deadlocks in kasan_meta_lock?
>>
>> If so, it is still unrelable because an OOB write can happen in
>> non-instrumented code. Or, kasan_meta_lock can successfully lock
>> overwritten garbage before noticing KASAN_KMALLOC_BAD_META. Or, two
>> threads can assume lock ownership after noticing
>> KASAN_KMALLOC_BAD_META.
>>
>> After the first report we continue working in kind of best effort
>> mode: we can try to mitigate some things, but generally all bets are
>> off. Because of that there is no need to build something complex,
>> global (and still unrelable). I would just wait for at most, say, 10
>> seconds in kasan_meta_lock, if we can't get the lock -- print an error
>> and return. That's simple, local and won't deadlock under any
>> circumstances.
>> The error message will be helpful, because there are chances we will
>> report a double-free on free of the corrupted object.
>> e
>> Tests can be arranged so that they write 0 (unlocked) into the meta
>> (if necessary).
>
> Dmitry,
>
> Thanks very much for review & comments. Yes, the locking scheme in v3
> is flawed in the presence of OOB writes on header, safety valve
> notwithstanding. The core issue is that when thread finds lock held, it is
> difficult to tell whether a legit lock holder exists or lock bit got flipped
> from OOB. Earlier, I did consider a lock timeout but felt it to be a bit ugly...
>
> However, I believe I've found a solution and was about to push out v4
> when your comments came in. It takes concept from v3 - exploiting
> shadow memory - to make lock much more reliable/resilient even in the
> presence of OOB writes. I'll push out v4 within the hour...
Locking shadow will probably work. Need to think more.
>> > + switch (alloc_info->state) {
>> > case KASAN_STATE_QUARANTINE:
>> > case KASAN_STATE_FREE:
>> > - pr_err("Double free");
>> > - dump_stack();
>> > - break;
>> > + kasan_report((unsigned long)object, 0, false, caller);
>> > + kasan_meta_unlock(alloc_info);
>> > + return true;
>> > default:
>>
>> Please at least print some here (it is not meant to happen, right?).
>
> ok.
>
>> > struct kasan_alloc_meta {
>> > + union {
>> > + u64 data;
>> > + struct {
>> > + u32 lock : 1; /* lock bit */
>>
>>
>> Add a comment that kasan_meta_lock expects this to be the first bit.
>
> Not required in v4...
>
> Thank you, once again.
>
> Kuthonuzo
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>