On Tue, Mar 21, 2017 at 11:41 AM, Mark Rutland [off-list ref] wrote:

On Tue, Mar 21, 2017 at 12:25:06PM +0300, Andrey Ryabinin wrote:

quoted

On 03/20/2017 08:17 PM, Mark Rutland wrote:

quoted

Hi,

On Tue, Mar 14, 2017 at 08:24:13PM +0100, Dmitry Vyukov wrote:

quoted

 /**
- * atomic_read - read atomic variable
+ * arch_atomic_read - read atomic variable
  * @v: pointer of type atomic_t
  *
  * Atomically reads the value of @v.
  */
-static __always_inline int atomic_read(const atomic_t *v)
+static __always_inline int arch_atomic_read(const atomic_t *v)
 {
-  return READ_ONCE((v)->counter);
+  /*
+   * We use READ_ONCE_NOCHECK() because atomic_read() contains KASAN
+   * instrumentation. Double instrumentation is unnecessary.
+   */
+  return READ_ONCE_NOCHECK((v)->counter);
 }

Just to check, we do this to avoid duplicate reports, right?

If so, double instrumentation isn't solely "unnecessary"; it has a
functional difference, and we should explicitly describe that in the
comment.

... or are duplicate reports supressed somehow?

They are not suppressed yet. But I think we should just switch kasan
to single shot mode, i.e. report only the first error. Single bug
quite often has multiple invalid memory accesses causing storm in
dmesg. Also write OOB might corrupt metadata so the next report will
print bogus alloc/free stacktraces.
In most cases we need to look only at the first report, so reporting
anything after the first is just counterproductive.

FWIW, that sounds sane to me.

Given that, I agree with your comment regarding READ_ONCE{,_NOCHECK}().

If anyone really wants all the reports, we could have a boot-time option
to do that.

I don't mind changing READ_ONCE_NOCHECK to READ_ONCE. But I don't have
strong preference either way.

We could do:
#define arch_atomic_read_is_already_instrumented 1
and then skip instrumentation in asm-generic if it's defined. But I
don't think it's worth it.

There is no functional difference, it's only an optimization (now
somewhat questionable). As Andrey said, one can get a splash of
reports anyway, and it's the first one that is important. We use KASAN
with panic_on_warn=1 so we don't even see the rest.

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