linux-kernel - Re: [PATCH v7 18/38] mm: slub: Disable KMSAN when checking the padding bytes

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] [day] [month] [year] [list]

Date: Mon, 24 Jun 2024 09:34:38 +0200
From: Vlastimil Babka <vbabka@...e.cz>
To: Ilya Leoshkevich <iii@...ux.ibm.com>,
 Alexander Gordeev <agordeev@...ux.ibm.com>,
 Alexander Potapenko <glider@...gle.com>,
 Andrew Morton <akpm@...ux-foundation.org>, Christoph Lameter <cl@...ux.com>,
 David Rientjes <rientjes@...gle.com>, Heiko Carstens <hca@...ux.ibm.com>,
 Joonsoo Kim <iamjoonsoo.kim@....com>, Marco Elver <elver@...gle.com>,
 Masami Hiramatsu <mhiramat@...nel.org>, Pekka Enberg <penberg@...nel.org>,
 Steven Rostedt <rostedt@...dmis.org>, Vasily Gorbik <gor@...ux.ibm.com>
Cc: Christian Borntraeger <borntraeger@...ux.ibm.com>,
 Dmitry Vyukov <dvyukov@...gle.com>, Hyeonggon Yoo <42.hyeyoo@...il.com>,
 kasan-dev@...glegroups.com, linux-kernel@...r.kernel.org,
 linux-mm@...ck.org, linux-s390@...r.kernel.org,
 linux-trace-kernel@...r.kernel.org, Mark Rutland <mark.rutland@....com>,
 Roman Gushchin <roman.gushchin@...ux.dev>,
 Sven Schnelle <svens@...ux.ibm.com>
Subject: Re: [PATCH v7 18/38] mm: slub: Disable KMSAN when checking the
 padding bytes

On 6/21/24 1:35 PM, Ilya Leoshkevich wrote:
> Even though the KMSAN warnings generated by memchr_inv() are suppressed
> by metadata_access_enable(), its return value may still be poisoned.
> 
> The reason is that the last iteration of memchr_inv() returns
> `*start != value ? start : NULL`, where *start is poisoned. Because of
> this, somewhat counterintuitively, the shadow value computed by
> visitSelectInst() is equal to `(uintptr_t)start`.
> 
> One possibility to fix this, since the intention behind guarding
> memchr_inv() behind metadata_access_enable() is to touch poisoned
> metadata without triggering KMSAN, is to unpoison its return value.
> However, this approach is too fragile. So simply disable the KMSAN
> checks in the respective functions.
> 
> Reviewed-by: Alexander Potapenko <glider@...gle.com>
> Signed-off-by: Ilya Leoshkevich <iii@...ux.ibm.com>

Acked-by: Vlastimil Babka <vbabka@...e.cz>

> ---
>  mm/slub.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/slub.c b/mm/slub.c
> index b050e528112c..fcd68fcea4ab 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -1176,9 +1176,16 @@ static void restore_bytes(struct kmem_cache *s, char *message, u8 data,
>  	memset(from, data, to - from);
>  }
>  
> -static int check_bytes_and_report(struct kmem_cache *s, struct slab *slab,
> -			u8 *object, char *what,
> -			u8 *start, unsigned int value, unsigned int bytes)
> +#ifdef CONFIG_KMSAN
> +#define pad_check_attributes noinline __no_kmsan_checks
> +#else
> +#define pad_check_attributes
> +#endif
> +
> +static pad_check_attributes int
> +check_bytes_and_report(struct kmem_cache *s, struct slab *slab,
> +		       u8 *object, char *what,
> +		       u8 *start, unsigned int value, unsigned int bytes)
>  {
>  	u8 *fault;
>  	u8 *end;
> @@ -1270,7 +1277,8 @@ static int check_pad_bytes(struct kmem_cache *s, struct slab *slab, u8 *p)
>  }
>  
>  /* Check the pad bytes at the end of a slab page */
> -static void slab_pad_check(struct kmem_cache *s, struct slab *slab)
> +static pad_check_attributes void
> +slab_pad_check(struct kmem_cache *s, struct slab *slab)
>  {
>  	u8 *start;
>  	u8 *fault;