Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) client-ip=2620:137:e000::3:2;
MIME-Version: 1.0
References: <20230914080833.50026-1-haibo.li@mediatek.com> <20230914112915.81f55863c0450195b4ed604a@linux-foundation.org>
In-Reply-To: <20230914112915.81f55863c0450195b4ed604a@linux-foundation.org>
From:   Andrey Konovalov <andreyknvl@gmail.com>
Date:   Thu, 14 Sep 2023 22:34:51 +0200
Message-ID: <CA+fCnZemM-jJxX+=2W162NJkUC6aZXNJiVLa-=ia=L3CmE8ZTQ@mail.gmail.com>
Subject: Re: [PATCH] kasan:fix access invalid shadow address when input is illegal
To:     Andrew Morton <akpm@linux-foundation.org>
Cc:     Haibo Li <haibo.li@mediatek.com>, 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, Jann Horn <jannh@google.com>,
        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 8:29=E2=80=AFPM Andrew Morton <akpm@linux-foundatio=
n.org> wrote:
> > --- a/mm/kasan/kasan.h
> > +++ b/mm/kasan/kasan.h
> > @@ -304,8 +304,17 @@ static __always_inline bool addr_has_metadata(cons=
t void *addr)
> >  #ifdef __HAVE_ARCH_SHADOW_MAP
> >       return (kasan_mem_to_shadow((void *)addr) !=3D NULL);
> >  #else
> > -     return (kasan_reset_tag(addr) >=3D
> > -             kasan_shadow_to_mem((void *)KASAN_SHADOW_START));
> > +     u8 *shadow, shadow_val;
> > +
> > +     if (kasan_reset_tag(addr) <
> > +             kasan_shadow_to_mem((void *)KASAN_SHADOW_START))
> > +             return false;
> > +     /* use read with nofault to check whether the shadow is accessibl=
e */
> > +     shadow =3D kasan_mem_to_shadow((void *)addr);
> > +     __get_kernel_nofault(&shadow_val, shadow, u8, fault);
> > +     return true;
> > +fault:
> > +     return false;
> >  #endif
> >  }
>
> Are we able to identify a Fixes: target for this?
> 9d7b7dd946924de43021f57a8bee122ff0744d93 ("kasan: split out
> print_report from __kasan_report") altered the code but I expect the
> bug was present before that commit.
>
> Seems this bug has been there for over a year.  Can you suggest why it
> has been discovered after such a lengthy time?

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.