[PATCH 3/3] arm64: kasan: mte: remove redundant mte_report_once logic

[Catalin Marinas]

On Sat, Jul 31, 2021 at 01:28:53PM +0200, Andrey Konovalov wrote:
> On Sat, Jul 31, 2021 at 12:44 PM Catalin Marinas
> <catalin.marinas at arm.com> wrote:
> > On Sat, Jul 31, 2021 at 01:01:25AM +0200, Andrey Konovalov wrote:
> > > On Tue, Jul 27, 2021 at 8:54 PM Catalin Marinas <catalin.marinas at arm.com> wrote:
> > > > On Wed, Jul 14, 2021 at 03:38:43PM +0100, Mark Rutland wrote:
> > > > > We have special logic to suppress MTE tag check fault reporting, based
> > > > > on a global `mte_report_once` and `reported` variables. These can be
> > > > > used to suppress calling kasan_report() when taking a tag check fault,
> > > > > but do not prevent taking the fault in the first place, nor does they
> > > > > affect the way we disable tag checks upon taking a fault.
> > > > >
> > > > > The core KASAN code already defaults to reporting a single fault, and
> > > > > has a `multi_shot` control to permit reporting multiple faults. The only
> > > > > place we transiently alter `mte_report_once` is in lib/test_kasan.c,
> > > > > where we also the `multi_shot` state as the same time. Thus
> > > > > `mte_report_once` and `reported` are redundant, and can be removed.
> > > > >
> > > > > When a tag check fault is taken, tag checking will be disabled by
> > > > > `do_tag_recovery` and must be explicitly re-enabled if desired. The test
> > > > > code does this by calling kasan_enable_tagging_sync().
> > > > >
> > > > > This patch removes the redundant mte_report_once() logic and associated
> > > > > variables.
> > > >
> > > > The first "reported" variable was added to avoid calling kasan_report on
> > > > each CPU as we are lazily disabling tag checking when faults are
> > > > triggered.
> > >
> > > So we do not need "reported" anymore? Why?
> >
> > Because kasan has its own mechanism with KASAN_BIT_REPORTED.
> 
> This mechanism only works when multi_shot is disabled. But milti_shot
> is always enabled when running the tests. Yet I tested the series, and
> I don't see multiple reports for each test, so everything somehow
> works. But I don't understand how.

When running tests, "reported" variable update is also disabled, so it
wouldn't make any difference.

When a fault happens in synchronous mode, to be able to continue safely
the tag checking is disabled on the current CPU (and lazily on the other
CPUs). So you won't be able to get another report on that CPU anyway. In
the unlikely event that the test thread migrates to another CPU before
completion, it may trigger another tag check fault on the new CPU and a
report.

We probably don't see multiple reports during a single test because (a)
kasan tests have preemption disabled, (b) they are unlikely to migrate
during the short time they run or (c) the kasan tests only do a single
faulty access.

-- 
Catalin