From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out2.suse.de (smtp-out2.suse.de [IPv6:2001:67c:2178:6::1d]) by sourceware.org (Postfix) with ESMTPS id 36D4D3858D39 for ; Tue, 14 Feb 2023 09:27:11 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 36D4D3858D39 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out2.suse.de (Postfix) with ESMTP id 18E4B1FD93; Tue, 14 Feb 2023 09:27:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1676366830; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=o16M4Lfgo5VeChEfECjQ212NwnaKp28aFnJyNwJfJwA=; b=0TfRcEBKjsn554flAdAop27ryD+9KmcOlbX6bc4fxInK83fgNxzAfFTMp0D6fH6KgUrSns qW4GAtqEQOeZGIvnM6uFoA9bszcEfusI+/g1JBpQ8MF6GG+zlMPpp5vz2S4J9iI6B+bjVf WB5ntXEE+0NY0JadjUsL73+UajV/oiA= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1676366830; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=o16M4Lfgo5VeChEfECjQ212NwnaKp28aFnJyNwJfJwA=; b=A5xcb2QSl2u9MDvpIsZFkfMIYVufWfCwg6ksjD+70pj97hYS34KOv1PsGMWwBDPQZQkapU JcjlBWJyZfPm9iDQ== Received: from wotan.suse.de (wotan.suse.de [10.160.0.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id 0C0182C141; Tue, 14 Feb 2023 09:27:09 +0000 (UTC) Date: Tue, 14 Feb 2023 09:27:09 +0000 (UTC) From: Richard Biener To: Jakub Jelinek cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH] asan: Add --param=asan-kernel-mem-intrinsic-prefix= [PR108777] In-Reply-To: Message-ID: References: User-Agent: Alpine 2.22 (LSU 394 2020-01-19) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-5.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,KAM_SHORT,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Tue, 14 Feb 2023, Jakub Jelinek wrote: > Hi! > > While in the -fsanitize=address case libasan overloads memcpy, memset, > memmove and many other builtins, such that they are always instrumented, > Linux kernel for -fsanitize=kernel-address recently changed or is changing, > such that memcpy, memset and memmove actually aren't instrumented because > they are often used also from no_sanitize ("kernel-address") functions > and wants __{,hw,}asaN_{memcpy,memset,memmove} to be used instead > for the instrumented calls. See e.g. the https://lkml.org/lkml/2023/2/9/1182 > thread. Without appropriate support on the compiler side, that will mean > any time a kernel-address instrumented function (most of them) calls > memcpy/memset/memmove, they will not be instrumented and thus won't catch > kernel bugs. Apparently clang 15 has a param for this. > > The following patch implements the same (except it is a usual GCC --param, > not -mllvm argument) on the GCC side. I know this isn't a regression > bugfix, but given that -fsanitize=kernel-address has a single project that > uses it which badly wants this I think it would be worthwhile to make an > exception and get this into GCC 13 rather than waiting another year, it > won't affect non-kernel code, nor even the kernel unless the new parameter > is used. > > Bootstrapped/regtested on x86_64-linux and i686-linux and Marco has tested > it on the kernel, ok for trunk? OK. Thanks, Richard. > 2023-02-14 Jakub Jelinek > > PR sanitizer/108777 > * params.opt (-param=asan-kernel-mem-intrinsic-prefix=): New param. > * asan.h (asan_memfn_rtl): Declare. > * asan.cc (asan_memfn_rtls): New variable. > (asan_memfn_rtl): New function. > * builtins.cc (expand_builtin): If > param_asan_kernel_mem_intrinsic_prefix and function is > kernel-{,hw}address sanitized, emit calls to > __{,hw}asan_{memcpy,memmove,memset} rather than > {memcpy,memmove,memset}. Use sanitize_flags_p (SANITIZE_ADDRESS) > instead of flag_sanitize & SANITIZE_ADDRESS to check if > asan_intercepted_p functions shouldn't be expanded inline. > > * gcc.dg/asan/pr108777-1.c: New test. > * gcc.dg/asan/pr108777-2.c: New test. > * gcc.dg/asan/pr108777-3.c: New test. > * gcc.dg/asan/pr108777-4.c: New test. > * gcc.dg/asan/pr108777-5.c: New test. > * gcc.dg/asan/pr108777-6.c: New test. > * gcc.dg/completion-3.c: Adjust expected multiline output. > > --- gcc/params.opt.jj 2023-02-10 19:04:58.289014706 +0100 > +++ gcc/params.opt 2023-02-13 16:19:50.411101775 +0100 > @@ -50,6 +50,10 @@ Enable asan store operations protection. > Common Joined UInteger Var(param_asan_instrumentation_with_call_threshold) Init(7000) Param Optimization > Use callbacks instead of inline code if number of accesses in function becomes greater or equal to this number. > > +-param=asan-kernel-mem-intrinsic-prefix= > +Common Joined UInteger Var(param_asan_kernel_mem_intrinsic_prefix) Init(0) IntegerRange(0, 1) Param Optimization > +Prefix calls to memcpy, memset and memmove with __asan_ or __hwasan_ for -fsanitize=kernel-address or -fsanitize=kernel-hwaddress. > + > -param=asan-memintrin= > Common Joined UInteger Var(param_asan_memintrin) Init(1) IntegerRange(0, 1) Param Optimization > Enable asan builtin functions protection. > --- gcc/asan.h.jj 2023-01-02 09:32:26.721222635 +0100 > +++ gcc/asan.h 2023-02-13 16:45:14.475088159 +0100 > @@ -33,6 +33,7 @@ extern bool asan_expand_check_ifn (gimpl > extern bool asan_expand_mark_ifn (gimple_stmt_iterator *); > extern bool asan_expand_poison_ifn (gimple_stmt_iterator *, bool *, > hash_map &); > +extern rtx asan_memfn_rtl (tree); > > extern void hwasan_record_frame_init (); > extern void hwasan_record_stack_var (rtx, rtx, poly_int64, poly_int64); > --- gcc/asan.cc.jj 2023-02-02 10:54:44.326473507 +0100 > +++ gcc/asan.cc 2023-02-13 16:52:16.711015256 +0100 > @@ -391,6 +391,46 @@ asan_memintrin (void) > } > > > +/* Support for --param asan-kernel-mem-intrinsic-prefix=1. */ > +static GTY(()) rtx asan_memfn_rtls[3]; > + > +rtx > +asan_memfn_rtl (tree fndecl) > +{ > + int i; > + const char *f, *p; > + char buf[sizeof ("__hwasan_memmove")]; > + > + switch (DECL_FUNCTION_CODE (fndecl)) > + { > + case BUILT_IN_MEMCPY: i = 0; f = "memcpy"; break; > + case BUILT_IN_MEMSET: i = 1; f = "memset"; break; > + case BUILT_IN_MEMMOVE: i = 2; f = "memmove"; break; > + default: gcc_unreachable (); > + } > + if (asan_memfn_rtls[i] == NULL_RTX) > + { > + tree save_name = DECL_NAME (fndecl); > + tree save_assembler_name = DECL_ASSEMBLER_NAME (fndecl); > + rtx save_rtl = DECL_RTL (fndecl); > + if (flag_sanitize & SANITIZE_KERNEL_HWADDRESS) > + p = "__hwasan_"; > + else > + p = "__asan_"; > + strcpy (buf, p); > + strcat (buf, f); > + DECL_NAME (fndecl) = get_identifier (buf); > + DECL_ASSEMBLER_NAME_RAW (fndecl) = NULL_TREE; > + SET_DECL_RTL (fndecl, NULL_RTX); > + asan_memfn_rtls[i] = DECL_RTL (fndecl); > + DECL_NAME (fndecl) = save_name; > + DECL_ASSEMBLER_NAME_RAW (fndecl) = save_assembler_name; > + SET_DECL_RTL (fndecl, save_rtl); > + } > + return asan_memfn_rtls[i]; > +} > + > + > /* Checks whether section SEC should be sanitized. */ > > static bool > --- gcc/builtins.cc.jj 2023-02-02 10:54:44.330473449 +0100 > +++ gcc/builtins.cc 2023-02-13 16:46:42.127826612 +0100 > @@ -7326,7 +7326,24 @@ expand_builtin (tree exp, rtx target, rt > by ASan. */ > > enum built_in_function fcode = DECL_FUNCTION_CODE (fndecl); > - if ((flag_sanitize & SANITIZE_ADDRESS) && asan_intercepted_p (fcode)) > + if (param_asan_kernel_mem_intrinsic_prefix > + && sanitize_flags_p (SANITIZE_KERNEL_ADDRESS > + | SANITIZE_KERNEL_HWADDRESS)) > + switch (fcode) > + { > + rtx save_decl_rtl, ret; > + case BUILT_IN_MEMCPY: > + case BUILT_IN_MEMMOVE: > + case BUILT_IN_MEMSET: > + save_decl_rtl = DECL_RTL (fndecl); > + DECL_RTL (fndecl) = asan_memfn_rtl (fndecl); > + ret = expand_call (exp, target, ignore); > + DECL_RTL (fndecl) = save_decl_rtl; > + return ret; > + default: > + break; > + } > + if (sanitize_flags_p (SANITIZE_ADDRESS) && asan_intercepted_p (fcode)) > return expand_call (exp, target, ignore); > > /* When not optimizing, generate calls to library functions for a certain > --- gcc/testsuite/gcc.dg/asan/pr108777-1.c.jj 2023-02-13 17:49:23.139002484 +0100 > +++ gcc/testsuite/gcc.dg/asan/pr108777-1.c 2023-02-13 17:51:09.433479988 +0100 > @@ -0,0 +1,28 @@ > +/* PR sanitizer/108777 */ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fno-sanitize=all -fsanitize=kernel-address --param asan-kernel-mem-intrinsic-prefix=1" } */ > +/* { dg-final { scan-assembler "__asan_memcpy" } } */ > +/* { dg-final { scan-assembler "__asan_memset" } } */ > +/* { dg-final { scan-assembler "__asan_memmove" } } */ > + > +extern void *memcpy (void *, const void *, __SIZE_TYPE__); > +extern void *memmove (void *, const void *, __SIZE_TYPE__); > +extern void *memset (void *, int, __SIZE_TYPE__); > + > +void > +foo (void *p, void *q, int s) > +{ > + memcpy (p, q, s); > +} > + > +void > +bar (void *p, void *q, int s) > +{ > + memmove (p, q, s); > +} > + > +void > +baz (void *p, int c, int s) > +{ > + memset (p, c, s); > +} > --- gcc/testsuite/gcc.dg/asan/pr108777-2.c.jj 2023-02-13 17:49:27.215944098 +0100 > +++ gcc/testsuite/gcc.dg/asan/pr108777-2.c 2023-02-13 17:51:09.433479988 +0100 > @@ -0,0 +1,24 @@ > +/* PR sanitizer/108777 */ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fno-sanitize=all -fsanitize=kernel-address --param asan-kernel-mem-intrinsic-prefix=1" } */ > +/* { dg-final { scan-assembler "__asan_memcpy" } } */ > +/* { dg-final { scan-assembler "__asan_memset" } } */ > +/* { dg-final { scan-assembler "__asan_memmove" } } */ > + > +void > +foo (void *p, void *q, int s) > +{ > + __builtin_memcpy (p, q, s); > +} > + > +void > +bar (void *p, void *q, int s) > +{ > + __builtin_memmove (p, q, s); > +} > + > +void > +baz (void *p, int c, int s) > +{ > + __builtin_memset (p, c, s); > +} > --- gcc/testsuite/gcc.dg/asan/pr108777-3.c.jj 2023-02-13 17:49:30.683894408 +0100 > +++ gcc/testsuite/gcc.dg/asan/pr108777-3.c 2023-02-13 17:51:09.434479973 +0100 > @@ -0,0 +1,28 @@ > +/* PR sanitizer/108777 */ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fno-sanitize=all -fsanitize=kernel-address --param asan-kernel-mem-intrinsic-prefix=1" } */ > +/* { dg-final { scan-assembler-not "__asan_memcpy" } } */ > +/* { dg-final { scan-assembler-not "__asan_memset" } } */ > +/* { dg-final { scan-assembler-not "__asan_memmove" } } */ > + > +extern void *memcpy (void *, const void *, __SIZE_TYPE__); > +extern void *memmove (void *, const void *, __SIZE_TYPE__); > +extern void *memset (void *, int, __SIZE_TYPE__); > + > +__attribute__((no_sanitize("kernel-address"))) void > +foo (void *p, void *q, int s) > +{ > + memcpy (p, q, s); > +} > + > +__attribute__((no_sanitize("kernel-address"))) void > +bar (void *p, void *q, int s) > +{ > + memmove (p, q, s); > +} > + > +__attribute__((no_sanitize("kernel-address"))) void > +baz (void *p, int c, int s) > +{ > + memset (p, c, s); > +} > --- gcc/testsuite/gcc.dg/asan/pr108777-4.c.jj 2023-02-13 17:49:33.985847114 +0100 > +++ gcc/testsuite/gcc.dg/asan/pr108777-4.c 2023-02-13 17:51:09.434479973 +0100 > @@ -0,0 +1,24 @@ > +/* PR sanitizer/108777 */ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fno-sanitize=all -fsanitize=kernel-address --param asan-kernel-mem-intrinsic-prefix=1" } */ > +/* { dg-final { scan-assembler-not "__asan_memcpy" } } */ > +/* { dg-final { scan-assembler-not "__asan_memset" } } */ > +/* { dg-final { scan-assembler-not "__asan_memmove" } } */ > + > +__attribute__((no_sanitize("kernel-address"))) void > +foo (void *p, void *q, int s) > +{ > + __builtin_memcpy (p, q, s); > +} > + > +__attribute__((no_sanitize("kernel-address"))) void > +bar (void *p, void *q, int s) > +{ > + __builtin_memmove (p, q, s); > +} > + > +__attribute__((no_sanitize("kernel-address"))) void > +baz (void *p, int c, int s) > +{ > + __builtin_memset (p, c, s); > +} > --- gcc/testsuite/gcc.dg/asan/pr108777-5.c.jj 2023-02-13 17:49:37.195801146 +0100 > +++ gcc/testsuite/gcc.dg/asan/pr108777-5.c 2023-02-13 17:51:09.434479973 +0100 > @@ -0,0 +1,28 @@ > +/* PR sanitizer/108777 */ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fno-sanitize=all -fsanitize=kernel-address" } */ > +/* { dg-final { scan-assembler-not "__asan_memcpy" } } */ > +/* { dg-final { scan-assembler-not "__asan_memset" } } */ > +/* { dg-final { scan-assembler-not "__asan_memmove" } } */ > + > +extern void *memcpy (void *, const void *, __SIZE_TYPE__); > +extern void *memmove (void *, const void *, __SIZE_TYPE__); > +extern void *memset (void *, int, __SIZE_TYPE__); > + > +void > +foo (void *p, void *q, int s) > +{ > + memcpy (p, q, s); > +} > + > +void > +bar (void *p, void *q, int s) > +{ > + memmove (p, q, s); > +} > + > +void > +baz (void *p, int c, int s) > +{ > + memset (p, c, s); > +} > --- gcc/testsuite/gcc.dg/asan/pr108777-6.c.jj 2023-02-13 17:49:40.282756931 +0100 > +++ gcc/testsuite/gcc.dg/asan/pr108777-6.c 2023-02-13 17:51:09.434479973 +0100 > @@ -0,0 +1,24 @@ > +/* PR sanitizer/108777 */ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fno-sanitize=all -fsanitize=kernel-address" } */ > +/* { dg-final { scan-assembler-not "__asan_memcpy" } } */ > +/* { dg-final { scan-assembler-not "__asan_memset" } } */ > +/* { dg-final { scan-assembler-not "__asan_memmove" } } */ > + > +void > +foo (void *p, void *q, int s) > +{ > + __builtin_memcpy (p, q, s); > +} > + > +void > +bar (void *p, void *q, int s) > +{ > + __builtin_memmove (p, q, s); > +} > + > +void > +baz (void *p, int c, int s) > +{ > + __builtin_memset (p, c, s); > +} > --- gcc/testsuite/gcc.dg/completion-3.c.jj 2020-01-14 20:02:47.249602853 +0100 > +++ gcc/testsuite/gcc.dg/completion-3.c 2023-02-14 09:39:44.613203143 +0100 > @@ -7,6 +7,7 @@ > --param=asan-instrument-reads= > --param=asan-instrument-writes= > --param=asan-instrumentation-with-call-threshold= > +--param=asan-kernel-mem-intrinsic-prefix= > --param=asan-memintrin= > --param=asan-stack= > --param=asan-use-after-return= > > Jakub > > -- Richard Biener SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman; HRB 36809 (AG Nuernberg)