Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
MIME-Version: 1.0
References: <20230914080833.50026-1-haibo.li@mediatek.com> <20230914112915.81f55863c0450195b4ed604a@linux-foundation.org>
 <CA+fCnZemM-jJxX+=2W162NJkUC6aZXNJiVLa-=ia=L3CmE8ZTQ@mail.gmail.com> <CAG48ez0aenPmr=d35UGa4_BiCwYU1-JHhD_2ygThvjOEXEM7bQ@mail.gmail.com>
In-Reply-To: <CAG48ez0aenPmr=d35UGa4_BiCwYU1-JHhD_2ygThvjOEXEM7bQ@mail.gmail.com>
From:   Andrey Konovalov <andreyknvl@gmail.com>
Date:   Fri, 15 Sep 2023 03:51:37 +0200
Message-ID: <CA+fCnZePgv=V65t4FtJvcyKvhM6yA3amTbPnwc5Ft5YdzpeeRg@mail.gmail.com>
Subject: Re: [PATCH] kasan:fix access invalid shadow address when input is illegal
To:     Jann Horn <jannh@google.com>, Haibo Li <haibo.li@mediatek.com>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        linux-kernel@vger.kernel.org, xiaoming.yu@mediatek.com,
        Andrey Ryabinin <ryabinin.a.a@gmail.com>,
        Alexander Potapenko <glider@google.com>,
        Dmitry Vyukov <dvyukov@google.com>,
        Vincenzo Frascino <vincenzo.frascino@arm.com>,
        Matthias Brugger <matthias.bgg@gmail.com>,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@collabora.com>,
        kasan-dev@googlegroups.com, linux-mm@kvack.org,
        linux-arm-kernel@lists.infradead.org,
        linux-mediatek@lists.infradead.org,
        Mark Rutland <mark.rutland@arm.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, Sep 14, 2023 at 10:41=E2=80=AFPM Jann Horn <jannh@google.com> wrote=
:
>
> > Accessing unmapped memory with KASAN always led to a crash when
> > checking shadow memory. This was reported/discussed before. To improve
> > crash reporting for this case, Jann added kasan_non_canonical_hook and
> > Mark integrated it into arm64. But AFAIU, for some reason, it stopped
> > working.
> >
> > Instead of this patch, we need to figure out why
> > kasan_non_canonical_hook stopped working and fix it.
> >
> > This approach taken by this patch won't work for shadow checks added
> > by compiler instrumentation. It only covers explicitly checked
> > accesses, such as via memcpy, etc.
>
> FWIW, AFAICS kasan_non_canonical_hook() currently only does anything
> under CONFIG_KASAN_INLINE;

Ah, right. I was thinking about the inline mode, but the patch refers
to the issue with the outline mode.

However, I just checked kasan_non_canonical_hook for SW_TAGS with the
inline mode: it does not work when accessing 0x42ffffb80aaaaaaa, the
addr < KASAN_SHADOW_OFFSET check fails. It appears there's something
unusual about how instrumentation calculates the shadow address. I
didn't investigate further yet.

> I think the idea when I added that was that
> it assumes that when KASAN checks an access in out-of-line
> instrumentation or a slowpath, it will do the required checks to avoid
> this kind of fault?

Ah, no, KASAN doesn't do it.

However, I suppose we could add what the original patch proposes for
the outline mode. For the inline mode, it seems to be pointless, as
most access checks happen though the compiler inserted code anyway.

I also wonder how much slowdown this patch will introduce.

Haibo, could you check how much slower the kernel becomes with your
patch? If possible, with all GENERIC/SW_TAGS and INLINE/OUTLINE
combinations.

If the slowdown is large, we can just make kasan_non_canonical_hook
work for both modes (and fix it for SW_TAGS).