On Sun, May 29, 2016 at 4:45 PM, Luruo, Kuthonuzo
[off-list ref] wrote:

quoted

quoted

+/* 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.

quoted

quoted

+       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.

quoted

quoted

 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@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>