linux-kernel - Re: [PATCH v6 0/4] x86, kasan: add KASAN checks to atomic operations

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180130153609.GA10917@arm.com>
Date:   Tue, 30 Jan 2018 15:36:10 +0000
From:   Will Deacon <will.deacon@....com>
To:     Dmitry Vyukov <dvyukov@...gle.com>
Cc:     mark.rutland@....com, peterz@...radead.org, mingo@...hat.com,
        hpa@...or.com, aryabinin@...tuozzo.com, kasan-dev@...glegroups.com,
        x86@...nel.org, linux-kernel@...r.kernel.org, tglx@...utronix.de
Subject: Re: [PATCH v6 0/4] x86, kasan: add KASAN checks to atomic operations

Hi Dmitry,

On Mon, Jan 29, 2018 at 06:26:03PM +0100, Dmitry Vyukov wrote:
> KASAN uses compiler instrumentation to intercept all memory accesses.
> But it does not see memory accesses done in assembly code.
> One notable user of assembly code is atomic operations. Frequently,
> for example, an atomic reference decrement is the last access to an
> object and a good candidate for a racy use-after-free.
> 
> Atomic operations are defined in arch files, but KASAN instrumentation
> is required for several archs that support KASAN. Later we will need
> similar hooks for KMSAN (uninit use detector) and KTSAN (data race
> detector).
> 
> This change introduces wrappers around atomic operations that can be
> used to add KASAN/KMSAN/KTSAN instrumentation across several archs,
> and adds KASAN checks to them.
> 
> This patch uses the wrappers only for x86 arch. Arm64 will be switched
> later. And we also plan to instrument bitops in a similar way.

One way you could reduce the intrusivness for each architecture would be
to leave the existing macro names as-is, and redefine them in the
asm-generic header. It's certainly ugly, but it makes the porting work
a lot smaller. Apologies if you've considered this approach before, but
I figured it was worth mentioning just in case.

e.g. for atomic[64]_read, your asm-generic header looks like:

#ifndef _LINUX_ATOMIC_INSTRUMENTED_H
#define _LINUX_ATOMIC_INSTRUMENTED_H

#include <linux/build_bug.h>
#include <linux/kasan-checks.h>

static __always_inline int __atomic_read_instrumented(const atomic_t *v)
{
	kasan_check_read(v, sizeof(*v));
	return atomic_read(v);
}

static __always_inline s64 __atomic64_read_instrumented(const atomic64_t *v)
{
	kasan_check_read(v, sizeof(*v));
	return atomic64_read(v);
}

#undef atomic_read
#undef atomic64_read

#define atomic_read	__atomic_read_instrumented
#define atomic64_read	__atomic64_read_instrumented

#endif /* _LINUX_ATOMIC_INSTRUMENTED_H */

and the arch code just includes that in asm/atomic.h once it's done with
its definitions.

What do you think? Too stinky?

Will