linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Vincenzo Frascino <vincenzo.frascino@arm.com>
To: Catalin Marinas <catalin.marinas@arm.com>
Cc: linux-arm-kernel@lists.infradead.org,
	Marco Elver <elver@google.com>,
	Elena Petrova <lenaptr@google.com>,
	Andrey Konovalov <andreyknvl@google.com>,
	Kevin Brodsky <kevin.brodsky@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	Branislav Rankov <Branislav.Rankov@arm.com>,
	kasan-dev@googlegroups.com, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org, Alexander Potapenko <glider@google.com>,
	Evgenii Stepanov <eugenis@google.com>,
	Andrey Ryabinin <aryabinin@virtuozzo.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Dmitry Vyukov <dvyukov@google.com>
Subject: Re: [PATCH 20/35] arm64: mte: Add in-kernel MTE helpers
Date: Thu, 27 Aug 2020 12:24:58 +0100	[thread overview]
Message-ID: <921c4ed0-b5b5-bc01-5418-c52d80f1af59@arm.com> (raw)
In-Reply-To: <20200827111027.GJ29264@gaia>



On 8/27/20 12:10 PM, Catalin Marinas wrote:
> On Thu, Aug 27, 2020 at 11:31:56AM +0100, Vincenzo Frascino wrote:
>> On 8/27/20 10:38 AM, Catalin Marinas wrote:
>>> On Fri, Aug 14, 2020 at 07:27:02PM +0200, Andrey Konovalov wrote:
>>>> +void * __must_check mte_set_mem_tag_range(void *addr, size_t size, u8 tag)
>>>> +{
>>>> +	void *ptr = addr;
>>>> +
>>>> +	if ((!system_supports_mte()) || (size == 0))
>>>> +		return addr;
>>>> +
>>>> +	tag = 0xF0 | (tag & 0xF);
>>>> +	ptr = (void *)__tag_set(ptr, tag);
>>>> +	size = ALIGN(size, MTE_GRANULE_SIZE);
>>>
>>> I think aligning the size is dangerous. Can we instead turn it into a
>>> WARN_ON if not already aligned? At a quick look, the callers of
>>> kasan_{un,}poison_memory() already align the size.
>>
>> The size here is used only for tagging purposes and if we want to tag a
>> subgranule amount of memory we end up tagging the granule anyway. Why do you
>> think it can be dangerous?
> 
> In principle, I don't like expanding the size unless you are an
> allocator. Since this code doesn't control the placement of the object
> it was given, a warn seems more appropriate.
> 

That's a good point. Ok, we can change this in a warning.

>>>> +/*
>>>> + * Assign allocation tags for a region of memory based on the pointer tag
>>>> + *   x0 - source pointer
>>>> + *   x1 - size
>>>> + *
>>>> + * Note: size is expected to be MTE_GRANULE_SIZE aligned
>>>> + */
>>>> +SYM_FUNC_START(mte_assign_mem_tag_range)
>>>> +	/* if (src == NULL) return; */
>>>> +	cbz	x0, 2f
>>>> +	/* if (size == 0) return; */
>>>
>>> You could skip the cbz here and just document that the size should be
>>> non-zero and aligned. The caller already takes care of this check.
>>
>> I would prefer to keep the check here, unless there is a valid reason, since
>> allocate(0) is a viable option hence tag(x, 0) should be as well. The caller
>> takes care of it in one place, today, but I do not know where the API will be
>> used in future.
> 
> That's why I said just document it in the comment above the function.
> 
> The check is also insufficient if the size is not aligned to an MTE
> granule, so it's not really consistent. This function should end with a
> subs followed by b.gt as cbnz will get stuck in a loop for unaligned
> size.
> 

That's correct. Thanks for pointing this out. I currently used it only in places
where the caller took care to align the size. But in future we cannot know hence
we should harden the function with what you are suggesting.

-- 
Regards,
Vincenzo

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

  reply	other threads:[~2020-08-27 11:24 UTC|newest]

Thread overview: 101+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-14 17:26 [PATCH 00/35] kasan: add hardware tag-based mode for arm64 Andrey Konovalov
2020-08-14 17:26 ` [PATCH 01/35] kasan: KASAN_VMALLOC depends on KASAN_GENERIC Andrey Konovalov
2020-08-14 17:26 ` [PATCH 02/35] kasan: group vmalloc code Andrey Konovalov
2020-08-14 17:26 ` [PATCH 03/35] kasan: shadow declarations only for software modes Andrey Konovalov
2020-09-18 14:55   ` Marco Elver
2020-09-18 14:56     ` Andrey Konovalov
2020-08-14 17:26 ` [PATCH 04/35] kasan: rename (un)poison_shadow to (un)poison_memory Andrey Konovalov
2020-08-14 17:26 ` [PATCH 05/35] kasan: rename KASAN_SHADOW_* to KASAN_GRANULE_* Andrey Konovalov
2020-08-14 17:26 ` [PATCH 06/35] kasan: only build init.c for software modes Andrey Konovalov
2020-08-14 17:26 ` [PATCH 07/35] kasan: split out shadow.c from common.c Andrey Konovalov
2020-08-14 17:26 ` [PATCH 08/35] kasan: rename generic/tags_report.c files Andrey Konovalov
2020-08-14 17:26 ` [PATCH 09/35] kasan: don't duplicate config dependencies Andrey Konovalov
2020-08-14 17:26 ` [PATCH 10/35] kasan: hide invalid free check implementation Andrey Konovalov
2020-08-14 17:26 ` [PATCH 11/35] kasan: decode stack frame only with KASAN_STACK_ENABLE Andrey Konovalov
2020-08-14 17:26 ` [PATCH 12/35] kasan, arm64: only init shadow for software modes Andrey Konovalov
2020-08-14 17:26 ` [PATCH 13/35] kasan, arm64: only use kasan_depth " Andrey Konovalov
2020-08-14 17:26 ` [PATCH 14/35] kasan: rename addr_has_shadow to addr_has_metadata Andrey Konovalov
2020-08-14 17:26 ` [PATCH 15/35] kasan: rename print_shadow_for_address to print_memory_metadata Andrey Konovalov
2020-08-14 17:26 ` [PATCH 16/35] kasan: kasan_non_canonical_hook only for software modes Andrey Konovalov
2020-08-14 17:26 ` [PATCH 17/35] kasan: rename SHADOW layout macros to META Andrey Konovalov
2020-08-14 17:27 ` [PATCH 18/35] kasan: separate metadata_fetch_row for each mode Andrey Konovalov
2020-08-14 17:27 ` [PATCH 19/35] kasan: don't allow SW_TAGS with ARM64_MTE Andrey Konovalov
2020-08-27  8:04   ` Catalin Marinas
2020-08-27  9:54     ` Vincenzo Frascino
2020-08-27 12:02       ` Andrey Konovalov
2020-08-14 17:27 ` [PATCH 20/35] arm64: mte: Add in-kernel MTE helpers Andrey Konovalov
2020-08-27  9:38   ` Catalin Marinas
2020-08-27 10:31     ` Vincenzo Frascino
2020-08-27 11:10       ` Catalin Marinas
2020-08-27 11:24         ` Vincenzo Frascino [this message]
2020-08-27 12:46     ` Andrey Konovalov
2020-09-08 13:23     ` Andrey Konovalov
2020-09-08 14:50       ` Catalin Marinas
2020-08-14 17:27 ` [PATCH 21/35] arm64: mte: Add in-kernel tag fault handler Andrey Konovalov
2020-08-27  9:54   ` Catalin Marinas
2020-08-27 10:44     ` Vincenzo Frascino
2020-08-27 12:31     ` Andrey Konovalov
2020-08-27 13:10       ` Catalin Marinas
2020-08-27 13:34         ` Andrey Konovalov
2020-08-27 14:56           ` Catalin Marinas
2020-08-27 19:14             ` Evgenii Stepanov
2020-08-28  9:56               ` Catalin Marinas
2020-08-14 17:27 ` [PATCH 22/35] arm64: mte: Enable in-kernel MTE Andrey Konovalov
2020-08-27 10:01   ` Catalin Marinas
2020-08-27 10:46     ` Vincenzo Frascino
2020-09-08 14:39   ` Andrey Konovalov
2020-09-08 14:52     ` Catalin Marinas
2020-08-14 17:27 ` [PATCH 23/35] arm64: mte: Convert gcr_user into an exclude mask Andrey Konovalov
2020-08-14 17:27 ` [PATCH 24/35] arm64: mte: Switch GCR_EL1 in kernel entry and exit Andrey Konovalov
2020-08-27 10:38   ` Catalin Marinas
2020-08-27 10:56     ` Vincenzo Frascino
2020-08-27 12:16       ` Catalin Marinas
2020-09-08 14:02         ` Andrey Konovalov
2020-09-08 14:53           ` Andrey Konovalov
2020-09-08 15:39           ` Catalin Marinas
2020-09-08 19:41             ` Derrick McKee
2020-09-08 13:58     ` Andrey Konovalov
2020-09-08 15:16       ` Catalin Marinas
2020-08-14 17:27 ` [PATCH 25/35] kasan: introduce CONFIG_KASAN_HW_TAGS Andrey Konovalov
2020-08-27 11:33   ` Vincenzo Frascino
2020-08-27 12:22     ` Andrey Konovalov
2020-08-14 17:27 ` [PATCH 26/35] kasan, arm64: Enable TBI EL1 Andrey Konovalov
2020-08-27 10:40   ` Catalin Marinas
2020-08-27 11:05     ` Vincenzo Frascino
2020-08-27 11:13       ` Catalin Marinas
2020-08-27 11:17         ` Vincenzo Frascino
2020-08-27 12:43           ` Andrey Konovalov
2020-08-27 13:45             ` Vincenzo Frascino
2020-08-27 14:36               ` Andrey Konovalov
2020-09-08 13:18     ` Andrey Konovalov
2020-09-08 14:06       ` Catalin Marinas
2020-09-08 14:12         ` Andrey Konovalov
2020-09-08 14:41           ` Catalin Marinas
2020-08-14 17:27 ` [PATCH 27/35] kasan, arm64: align allocations for HW_TAGS Andrey Konovalov
2020-08-14 17:27 ` [PATCH 28/35] kasan: define KASAN_GRANULE_SIZE " Andrey Konovalov
2020-08-27 10:41   ` Catalin Marinas
2020-08-27 11:07     ` Vincenzo Frascino
2020-08-27 12:05       ` Andrey Konovalov
2020-08-14 17:27 ` [PATCH 29/35] kasan, x86, s390: update undef CONFIG_KASAN Andrey Konovalov
2020-08-14 17:27 ` [PATCH 30/35] kasan, arm64: expand CONFIG_KASAN checks Andrey Konovalov
2020-08-14 17:27 ` [PATCH 31/35] kasan, arm64: implement HW_TAGS runtime Andrey Konovalov
2020-08-27 10:45   ` Catalin Marinas
2020-08-27 11:35     ` Vincenzo Frascino
2020-08-27 12:37     ` Andrey Konovalov
2020-08-14 17:27 ` [PATCH 32/35] kasan, arm64: print report from tag fault handler Andrey Konovalov
2020-08-27 10:48   ` Catalin Marinas
2020-08-27 12:11     ` Vincenzo Frascino
2020-08-27 12:34     ` Andrey Konovalov
2020-08-27 14:21       ` Catalin Marinas
2020-08-14 17:27 ` [PATCH 33/35] kasan, slub: reset tags when accessing metadata Andrey Konovalov
2020-08-14 17:27 ` [PATCH 34/35] kasan, arm64: enable CONFIG_KASAN_HW_TAGS Andrey Konovalov
2020-08-14 17:27 ` [PATCH 35/35] kasan: add documentation for hardware tag-based mode Andrey Konovalov
2020-08-28 11:12   ` Marco Elver
2020-08-28 12:28     ` Andrey Konovalov
2020-09-14 19:06 ` [PATCH 00/35] kasan: add hardware tag-based mode for arm64 Derrick McKee
2020-09-14 20:04   ` Andrey Konovalov
2020-09-14 21:36     ` Derrick McKee
2020-09-14 22:41       ` Derrick McKee
2020-09-14 22:50         ` Andrey Konovalov
2020-09-15  9:46           ` Derrick McKee
2020-09-15 17:51             ` Derrick McKee

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=921c4ed0-b5b5-bc01-5418-c52d80f1af59@arm.com \
    --to=vincenzo.frascino@arm.com \
    --cc=Branislav.Rankov@arm.com \
    --cc=akpm@linux-foundation.org \
    --cc=andreyknvl@google.com \
    --cc=aryabinin@virtuozzo.com \
    --cc=catalin.marinas@arm.com \
    --cc=dvyukov@google.com \
    --cc=elver@google.com \
    --cc=eugenis@google.com \
    --cc=glider@google.com \
    --cc=kasan-dev@googlegroups.com \
    --cc=kevin.brodsky@arm.com \
    --cc=lenaptr@google.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=will.deacon@arm.com \
    --subject='Re: [PATCH 20/35] arm64: mte: Add in-kernel MTE helpers' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).