linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Andrey Konovalov <andreyknvl@gmail.com>
To: "袁帅(Shuai Yuan)" <yuanshuai@zeku.com>,
	"Catalin Marinas" <catalin.marinas@arm.com>
Cc: "Dmitry Vyukov" <dvyukov@google.com>,
	"欧阳炜钊(Weizhao Ouyang)" <ouyangweizhao@zeku.com>,
	"Andrey Ryabinin" <ryabinin.a.a@gmail.com>,
	"Alexander Potapenko" <glider@google.com>,
	"Vincenzo Frascino" <vincenzo.frascino@arm.com>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"kasan-dev@googlegroups.com" <kasan-dev@googlegroups.com>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Weizhao Ouyang" <o451686892@gmail.com>,
	"任立鹏(Peng Ren)" <renlipeng@zeku.com>,
	"Peter Collingbourne" <pcc@google.com>
Subject: Re: [PATCH v2] kasan: fix deadlock in start_report()
Date: Mon, 27 Feb 2023 03:13:45 +0100	[thread overview]
Message-ID: <CA+fCnZcirNwdA=oaLLiDN+NxBPNcA75agPV1sRsKuZ0Wz6w_hQ@mail.gmail.com> (raw)
In-Reply-To: <2b57491a9fab4ce9a643bd0922e03e73@zeku.com>

On Wed, Feb 15, 2023 at 2:22 PM 袁帅(Shuai Yuan) <yuanshuai@zeku.com> wrote:
>
> I have got valid information to clarify the problem and solutions. I made
> a few changes to the code to do this.
>
> a) I was testing on a device that had hardware issues with MTE,
>     and the memory tag sometimes changed randomly.

Ah, I see. Faulty hardware explains the problem. Thank you!

> f) From the above log, you can see that the system tried to call kasan_report() twice,
>    because we visit tag address by kmem_cache and this tag have change..
>    Normally this doesn't happen easily. So I think we can add kasan_reset_tag() to handle
>    the kmem_cache address.
>
>    For example, the following changes are used for the latest kernel version.
> diff --git a/mm/kasan/report.c b/mm/kasan/report.c
> --- a/mm/kasan/report.c
> +++ b/mm/kasan/report.c
> @@ -412,7 +412,7 @@ static void complete_report_info(struct kasan_report_info *info)
>         slab = kasan_addr_to_slab(addr);
>         if (slab) {
> -               info->cache = slab->slab_cache;
> +               info->cache = kasan_reset_tag(slab->slab_cache);

This fixes the problem for accesses to slab_cache, but KASAN reporting
code also accesses stack depot memory and calls other routines that
might access (faulty) tagged memory. And the accessed addresses aren't
exposed to KASAN code, so we can't use kasan_reset_tag for those.

I wonder what would be a good solution here. I really don't want to
use kasan_depth or some other global/per-cpu flag here, as it would be
too good of a target for attackers wishing to bypass MTE. Perhaps,
disabling MTE once reporting started would be a better option: calling
the disabling routine would arguably be a harder task for an attacker
than overwriting a flag.

+Catalin, would it be acceptable to implement a routine that disables
in-kernel MTE tag checking (until the next
mte_enable_kernel_sync/async/asymm call)? In a similar way an MTE
fault does this, but without the fault itself. I.e., expose the part
of do_tag_recovery functionality without report_tag_fault?

TL;DR on the problem: Besides relying on CPU tag checks, KASAN also
does explicit tag checks to detect double-frees and similar problems,
see the calls to kasan_report_invalid_free. Thus, when e.g. a
double-free report is printed, MTE checking is still on. This results
in a deadlock in case invalid memory is accessed during KASAN
reporting.

Thanks!


  reply	other threads:[~2023-02-27  2:14 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20230209031159.2337445-1-ouyangweizhao@zeku.com>
2023-02-09  8:55 ` Dmitry Vyukov
     [not found]   ` <93b94f59016145adbb1e01311a1103f8@zeku.com>
2023-02-09 10:44     ` Dmitry Vyukov
2023-02-09 22:54       ` Andrey Konovalov
     [not found]         ` <b058a424e46d4f94a1f2fdc61292606b@zeku.com>
2023-02-15 13:22           ` 袁帅(Shuai Yuan)
2023-02-27  2:13             ` Andrey Konovalov [this message]
2023-02-28 16:09               ` Catalin Marinas
2023-02-28 21:50                 ` Andrey Konovalov
2023-03-01 16:59                   ` Catalin Marinas
2023-03-10 23:42                     ` Andrey Konovalov
2023-03-15 16:14                       ` Catalin Marinas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CA+fCnZcirNwdA=oaLLiDN+NxBPNcA75agPV1sRsKuZ0Wz6w_hQ@mail.gmail.com' \
    --to=andreyknvl@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=catalin.marinas@arm.com \
    --cc=dvyukov@google.com \
    --cc=glider@google.com \
    --cc=kasan-dev@googlegroups.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=o451686892@gmail.com \
    --cc=ouyangweizhao@zeku.com \
    --cc=pcc@google.com \
    --cc=renlipeng@zeku.com \
    --cc=ryabinin.a.a@gmail.com \
    --cc=vincenzo.frascino@arm.com \
    --cc=yuanshuai@zeku.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox