All of lore.kernel.org
 help / color / mirror / Atom feed
From: Catalin Marinas <catalin.marinas@arm.com>
To: Vincenzo Frascino <vincenzo.frascino@arm.com>
Cc: linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, kasan-dev@googlegroups.com,
	Andrew Morton <akpm@linux-foundation.org>,
	Will Deacon <will@kernel.org>, Dmitry Vyukov <dvyukov@google.com>,
	Andrey Ryabinin <aryabinin@virtuozzo.com>,
	Alexander Potapenko <glider@google.com>,
	Marco Elver <elver@google.com>,
	Evgenii Stepanov <eugenis@google.com>,
	Branislav Rankov <Branislav.Rankov@arm.com>,
	Andrey Konovalov <andreyknvl@google.com>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Subject: Re: [PATCH v14 8/8] kselftest/arm64: Verify that TCO is enabled in load_unaligned_zeropad()
Date: Thu, 11 Mar 2021 16:28:21 +0000	[thread overview]
Message-ID: <20210311162820.GE30821@arm.com> (raw)
In-Reply-To: <bd403b9f-bb38-a456-b176-b6fefccb711f@arm.com>

On Thu, Mar 11, 2021 at 03:00:26PM +0000, Vincenzo Frascino wrote:
> On 3/11/21 1:25 PM, Catalin Marinas wrote:
> > On Mon, Mar 08, 2021 at 04:14:34PM +0000, Vincenzo Frascino wrote:
> >> load_unaligned_zeropad() and __get/put_kernel_nofault() functions can
> >> read passed some buffer limits which may include some MTE granule with a
> >> different tag.
> >>
> >> When MTE async mode is enable, the load operation crosses the boundaries
> >> and the next granule has a different tag the PE sets the TFSR_EL1.TF1
> >> bit as if an asynchronous tag fault is happened:
> >>
> >>  ==================================================================
> >>  BUG: KASAN: invalid-access
> >>  Asynchronous mode enabled: no access details available
> >>
> >>  CPU: 0 PID: 1 Comm: init Not tainted 5.12.0-rc1-ge1045c86620d-dirty #8
> >>  Hardware name: FVP Base RevC (DT)
> >>  Call trace:
> >>    dump_backtrace+0x0/0x1c0
> >>    show_stack+0x18/0x24
> >>    dump_stack+0xcc/0x14c
> >>    kasan_report_async+0x54/0x70
> >>    mte_check_tfsr_el1+0x48/0x4c
> >>    exit_to_user_mode+0x18/0x38
> >>    finish_ret_to_user+0x4/0x15c
> >>  ==================================================================
> >>
> >> Verify that Tag Check Override (TCO) is enabled in these functions before
> >> the load and disable it afterwards to prevent this to happen.
> >>
> >> Note: The issue has been observed only with an MTE enabled userspace.
> > 
> > The above bug is all about kernel buffers. While userspace can trigger
> > the relevant code paths, it should not matter whether the user has MTE
> > enabled or not. Can you please confirm that you can still triggered the
> > fault with kernel-mode MTE but non-MTE user-space? If not, we may have a
> > bug somewhere as the two are unrelated: load_unaligned_zeropad() only
> > acts on kernel buffers and are subject to the kernel MTE tag check fault
> > mode.
> 
> I retried and you are right, it does not matter if it is a MTE or non-MTE
> user-space. The issue seems to be that this test does not trigger the problem
> all the times which probably lead me to the wrong conclusions.

Keep the test around for some quick checks before you get the kasan
test support.

> > I don't think we should have a user-space selftest for this. The bug is
> > not about a user-kernel interface, so an in-kernel test is more
> > appropriate. Could we instead add this to the kasan tests and calling
> > load_unaligned_zeropad() and other functions directly?
> 
> I agree with you we should abandon this strategy of triggering the issue due to
> my comment above. I will investigate the option of having a kasan test and try
> to come up with one that calls the relevant functions directly. I would prefer
> though, since the rest of the series is almost ready, to post it in a future
> series. What do you think?

That's fine by me.

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Catalin Marinas <catalin.marinas@arm.com>
To: Vincenzo Frascino <vincenzo.frascino@arm.com>
Cc: linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, kasan-dev@googlegroups.com,
	Andrew Morton <akpm@linux-foundation.org>,
	Will Deacon <will@kernel.org>, Dmitry Vyukov <dvyukov@google.com>,
	Andrey Ryabinin <aryabinin@virtuozzo.com>,
	Alexander Potapenko <glider@google.com>,
	Marco Elver <elver@google.com>,
	Evgenii Stepanov <eugenis@google.com>,
	Branislav Rankov <Branislav.Rankov@arm.com>,
	Andrey Konovalov <andreyknvl@google.com>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Subject: Re: [PATCH v14 8/8] kselftest/arm64: Verify that TCO is enabled in load_unaligned_zeropad()
Date: Thu, 11 Mar 2021 16:28:21 +0000	[thread overview]
Message-ID: <20210311162820.GE30821@arm.com> (raw)
In-Reply-To: <bd403b9f-bb38-a456-b176-b6fefccb711f@arm.com>

On Thu, Mar 11, 2021 at 03:00:26PM +0000, Vincenzo Frascino wrote:
> On 3/11/21 1:25 PM, Catalin Marinas wrote:
> > On Mon, Mar 08, 2021 at 04:14:34PM +0000, Vincenzo Frascino wrote:
> >> load_unaligned_zeropad() and __get/put_kernel_nofault() functions can
> >> read passed some buffer limits which may include some MTE granule with a
> >> different tag.
> >>
> >> When MTE async mode is enable, the load operation crosses the boundaries
> >> and the next granule has a different tag the PE sets the TFSR_EL1.TF1
> >> bit as if an asynchronous tag fault is happened:
> >>
> >>  ==================================================================
> >>  BUG: KASAN: invalid-access
> >>  Asynchronous mode enabled: no access details available
> >>
> >>  CPU: 0 PID: 1 Comm: init Not tainted 5.12.0-rc1-ge1045c86620d-dirty #8
> >>  Hardware name: FVP Base RevC (DT)
> >>  Call trace:
> >>    dump_backtrace+0x0/0x1c0
> >>    show_stack+0x18/0x24
> >>    dump_stack+0xcc/0x14c
> >>    kasan_report_async+0x54/0x70
> >>    mte_check_tfsr_el1+0x48/0x4c
> >>    exit_to_user_mode+0x18/0x38
> >>    finish_ret_to_user+0x4/0x15c
> >>  ==================================================================
> >>
> >> Verify that Tag Check Override (TCO) is enabled in these functions before
> >> the load and disable it afterwards to prevent this to happen.
> >>
> >> Note: The issue has been observed only with an MTE enabled userspace.
> > 
> > The above bug is all about kernel buffers. While userspace can trigger
> > the relevant code paths, it should not matter whether the user has MTE
> > enabled or not. Can you please confirm that you can still triggered the
> > fault with kernel-mode MTE but non-MTE user-space? If not, we may have a
> > bug somewhere as the two are unrelated: load_unaligned_zeropad() only
> > acts on kernel buffers and are subject to the kernel MTE tag check fault
> > mode.
> 
> I retried and you are right, it does not matter if it is a MTE or non-MTE
> user-space. The issue seems to be that this test does not trigger the problem
> all the times which probably lead me to the wrong conclusions.

Keep the test around for some quick checks before you get the kasan
test support.

> > I don't think we should have a user-space selftest for this. The bug is
> > not about a user-kernel interface, so an in-kernel test is more
> > appropriate. Could we instead add this to the kasan tests and calling
> > load_unaligned_zeropad() and other functions directly?
> 
> I agree with you we should abandon this strategy of triggering the issue due to
> my comment above. I will investigate the option of having a kasan test and try
> to come up with one that calls the relevant functions directly. I would prefer
> though, since the rest of the series is almost ready, to post it in a future
> series. What do you think?

That's fine by me.

-- 
Catalin

  reply	other threads:[~2021-03-11 16:30 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-08 16:14 [PATCH v14 0/8] arm64: ARMv8.5-A: MTE: Add async mode support Vincenzo Frascino
2021-03-08 16:14 ` Vincenzo Frascino
2021-03-08 16:14 ` [PATCH v14 1/8] arm64: mte: Add asynchronous " Vincenzo Frascino
2021-03-08 16:14   ` Vincenzo Frascino
2021-03-08 16:14 ` [PATCH v14 2/8] kasan: Add KASAN mode kernel parameter Vincenzo Frascino
2021-03-08 16:14   ` Vincenzo Frascino
2021-03-08 16:14 ` [PATCH v14 3/8] arm64: mte: Drop arch_enable_tagging() Vincenzo Frascino
2021-03-08 16:14   ` Vincenzo Frascino
2021-03-11 10:38   ` Catalin Marinas
2021-03-11 10:38     ` Catalin Marinas
2021-03-08 16:14 ` [PATCH v14 4/8] kasan: Add report for async mode Vincenzo Frascino
2021-03-08 16:14   ` Vincenzo Frascino
2021-03-08 16:14 ` [PATCH v14 5/8] arm64: mte: Enable TCO in functions that can read beyond buffer limits Vincenzo Frascino
2021-03-08 16:14   ` Vincenzo Frascino
2021-03-08 18:09   ` Mark Rutland
2021-03-08 18:09     ` Mark Rutland
2021-03-09 10:26     ` Vincenzo Frascino
2021-03-09 10:26       ` Vincenzo Frascino
2021-03-08 21:58   ` kernel test robot
2021-03-08 21:58     ` kernel test robot
2021-03-08 21:58     ` kernel test robot
2021-03-09  0:28   ` kernel test robot
2021-03-09  0:28     ` kernel test robot
2021-03-09  0:28     ` kernel test robot
2021-03-09 22:40   ` kernel test robot
2021-03-09 22:40     ` kernel test robot
2021-03-09 22:40     ` kernel test robot
2021-03-08 16:14 ` [PATCH v14 6/8] arm64: mte: Enable async tag check fault Vincenzo Frascino
2021-03-08 16:14   ` Vincenzo Frascino
2021-03-08 16:14 ` [PATCH v14 7/8] arm64: mte: Report async tag faults before suspend Vincenzo Frascino
2021-03-08 16:14   ` Vincenzo Frascino
2021-03-08 16:14 ` [PATCH v14 8/8] kselftest/arm64: Verify that TCO is enabled in load_unaligned_zeropad() Vincenzo Frascino
2021-03-08 16:14   ` Vincenzo Frascino
2021-03-11 13:25   ` Catalin Marinas
2021-03-11 13:25     ` Catalin Marinas
2021-03-11 15:00     ` Vincenzo Frascino
2021-03-11 15:00       ` Vincenzo Frascino
2021-03-11 16:28       ` Catalin Marinas [this message]
2021-03-11 16:28         ` Catalin Marinas
2021-03-11 16:34         ` Vincenzo Frascino
2021-03-11 16:34           ` Vincenzo Frascino
2021-03-08 21:02 ` [PATCH v14 0/8] arm64: ARMv8.5-A: MTE: Add async mode support Andrey Konovalov
2021-03-08 21:02   ` Andrey Konovalov
2021-03-09 10:11   ` Vincenzo Frascino
2021-03-09 10:11     ` Vincenzo Frascino

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=20210311162820.GE30821@arm.com \
    --to=catalin.marinas@arm.com \
    --cc=Branislav.Rankov@arm.com \
    --cc=akpm@linux-foundation.org \
    --cc=andreyknvl@google.com \
    --cc=aryabinin@virtuozzo.com \
    --cc=dvyukov@google.com \
    --cc=elver@google.com \
    --cc=eugenis@google.com \
    --cc=glider@google.com \
    --cc=kasan-dev@googlegroups.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=vincenzo.frascino@arm.com \
    --cc=will@kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.