MIME-Version: 1.0
References: <cover.1638308023.git.andreyknvl@google.com> <4fbc6668845e699bf708aee5c11ad9fd012d4dcd.1638308023.git.andreyknvl@google.com>
 <YajX7pyIK27Gd+IE@elver.google.com>
In-Reply-To: <YajX7pyIK27Gd+IE@elver.google.com>
From:   Andrey Konovalov <andreyknvl@gmail.com>
Date:   Mon, 6 Dec 2021 22:08:59 +0100
Message-ID: <CA+fCnZdWhnSDqtQ+q1RUV1U1uVtGpr0oxVK5jtUZUn=W+5rSjw@mail.gmail.com>
Subject: Re: [PATCH 21/31] kasan, fork: don't tag stacks allocated with vmalloc
To:     Marco Elver <elver@google.com>
Cc:     andrey.konovalov@linux.dev,
        Alexander Potapenko <glider@google.com>,
        Vincenzo Frascino <vincenzo.frascino@arm.com>,
        Catalin Marinas <catalin.marinas@arm.com>,
        Peter Collingbourne <pcc@google.com>,
        Dmitry Vyukov <dvyukov@google.com>,
        kasan-dev <kasan-dev@googlegroups.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Linux Memory Management List <linux-mm@kvack.org>,
        Will Deacon <will@kernel.org>,
        Linux ARM <linux-arm-kernel@lists.infradead.org>,
        Evgenii Stepanov <eugenis@google.com>,
        LKML <linux-kernel@vger.kernel.org>,
        Andrey Konovalov <andreyknvl@google.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Dec 2, 2021 at 3:28 PM Marco Elver <elver@google.com> wrote:
>
> On Tue, Nov 30, 2021 at 11:07PM +0100, andrey.konovalov@linux.dev wrote:
> > From: Andrey Konovalov <andreyknvl@google.com>
> >
> > Once tag-based KASAN modes start tagging vmalloc() allocations,
> > kernel stacks will start getting tagged if CONFIG_VMAP_STACK is enabled.
> >
> > Reset the tag of kernel stack pointers after allocation.
> >
> > For SW_TAGS KASAN, when CONFIG_KASAN_STACK is enabled, the
> > instrumentation can't handle the sp register being tagged.
> >
> > For HW_TAGS KASAN, there's no instrumentation-related issues. However,
> > the impact of having a tagged SP pointer needs to be properly evaluated,
> > so keep it non-tagged for now.
>
> Don't VMAP_STACK stacks have guards? So some out-of-bounds would already
> be caught.

True, linear out-of-bounds accesses are already caught.

> What would be the hypothetical benefit of using a tagged stack pointer?
> Perhaps wildly out-of-bounds accesses derived from stack pointers?

Yes, that's the case that comes to mind.

> I agree that unless we understand the impact of using a tagged stack
> pointers, it should remain non-tagged for now.

Ack. I'll file a KASAN bug for this when the series is merged.

> > Note, that the memory for the stack allocation still gets tagged to
> > catch vmalloc-into-stack out-of-bounds accesses.
>
> Will the fact it's tagged cause issues for other code? I think kmemleak
> already untags all addresses it scans for pointers. Anything else?

Tagging stack memory shouldn't cause any stability issues like
conflicts with kmemleak. Tagging memory but not the pointers is not
worse than leaving memory tags uninitialized/random with regards to
this kind of issues.

Thanks!