linux-kernel - Re: [PATCH v2 1/9] kasan: sw_tags: Use arithmetic shift for shadow computation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <zjuvfdbl7q76ahdxk3lrgaznk7vjj43f5ftzfgrnca6dqtcd5x@5qj24womzgyq>
Date: Mon, 10 Feb 2025 16:52:51 +0100
From: Maciej Wieczor-Retman <maciej.wieczor-retman@...el.com>
To: Andrey Konovalov <andreyknvl@...il.com>
CC: Samuel Holland <samuel.holland@...ive.com>, Palmer Dabbelt
	<palmer@...belt.com>, <linux-riscv@...ts.infradead.org>, Andrey Ryabinin
	<ryabinin.a.a@...il.com>, Alexander Potapenko <glider@...gle.com>, "Dmitry
 Vyukov" <dvyukov@...gle.com>, Vincenzo Frascino <vincenzo.frascino@....com>,
	<kasan-dev@...glegroups.com>, <llvm@...ts.linux.dev>, Catalin Marinas
	<catalin.marinas@....com>, <linux-kernel@...r.kernel.org>,
	<linux-mm@...ck.org>, Alexandre Ghiti <alexghiti@...osinc.com>, Will Deacon
	<will@...nel.org>, Evgenii Stepanov <eugenis@...gle.com>, Andrew Morton
	<akpm@...ux-foundation.org>, <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v2 1/9] kasan: sw_tags: Use arithmetic shift for shadow
 computation

On 2025-02-10 at 16:22:41 +0100, Maciej Wieczor-Retman wrote:
>On 2024-10-23 at 20:41:57 +0200, Andrey Konovalov wrote:
>>On Tue, Oct 22, 2024 at 3:59 AM Samuel Holland
>><samuel.holland@...ive.com> wrote:
>...
>>> +        * Software Tag-Based KASAN, the displacement is signed, so
>>> +        * KASAN_SHADOW_OFFSET is the center of the range.
>>>          */
>>> -       if (addr < KASAN_SHADOW_OFFSET)
>>> -               return;
>>> +       if (IS_ENABLED(CONFIG_KASAN_GENERIC)) {
>>> +               if (addr < KASAN_SHADOW_OFFSET ||
>>> +                   addr >= KASAN_SHADOW_OFFSET + max_shadow_size)
>>> +                       return;
>>> +       } else {
>>> +               if (addr < KASAN_SHADOW_OFFSET - max_shadow_size / 2 ||
>>> +                   addr >= KASAN_SHADOW_OFFSET + max_shadow_size / 2)
>>> +                       return;
>>
>>Hm, I might be wrong, but I think this check does not work.
>>
>>Let's say we have non-canonical address 0x4242424242424242 and number
>>of VA bits is 48.
>>
>>Then:
>>
>>KASAN_SHADOW_OFFSET == 0xffff800000000000
>>kasan_mem_to_shadow(0x4242424242424242) == 0x0423a42424242424
>>max_shadow_size == 0x1000000000000000
>>KASAN_SHADOW_OFFSET - max_shadow_size / 2 == 0xf7ff800000000000
>>KASAN_SHADOW_OFFSET + max_shadow_size / 2 == 0x07ff800000000000 (overflows)
>>
>>0x0423a42424242424 is < than 0xf7ff800000000000, so the function will
>>wrongly return.
>
>As I understand this check aims to figure out if the address landed in shadow
>space and if it didn't we can return.
>
>Can't this above snippet be a simple:
>
>	if (!addr_in_shadow(addr))
>		return;
>
>?

Sorry, I think this wouldn't work. The tag also needs to be reset. Does this
perhaps work for this problem?

	if (!addr_in_shadow(kasan_reset_tag((void *)addr)))
		return;

>
>-- 
>Kind regards
>Maciej Wieczór-Retman

-- 
Kind regards
Maciej Wieczór-Retman