Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 27528C636D4 for ; Mon, 13 Feb 2023 07:01:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229720AbjBMHBC (ORCPT ); Mon, 13 Feb 2023 02:01:02 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48208 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229468AbjBMHBA (ORCPT ); Mon, 13 Feb 2023 02:01:00 -0500 Received: from mail-vk1-xa29.google.com (mail-vk1-xa29.google.com [IPv6:2607:f8b0:4864:20::a29]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1C06F113EF for ; Sun, 12 Feb 2023 23:00:56 -0800 (PST) Received: by mail-vk1-xa29.google.com with SMTP id 9so5759096vkq.9 for ; Sun, 12 Feb 2023 23:00:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=Cw3Q2nDkJ9Bv8PVsAGDCbfG6m5PVGNiCmox+HLaI3GE=; b=qXgprL150OL9B4VJjN93H9lN+jbXJ1nIsMcEhvYQ3EODoACnRLkm5uDPATHWOMJixo DrRPLihx6Z3HvpC98A1g/N2LxGYXW59knPSPq+aUYDPri4LHjW7RnrqWCu8GXhXjX2p+ CgGkySH/C660taFg71aICkmWVwtbkCKlEWwsYw1eFuo+ePQ44uWQW36nR+p3oE7ngyw5 ZaSK5lnGO//sOwP9x6yyIotta8ZeGcPDpeAx4L3qbBjik2gPqBhy+srZLHvxzSX/4hk8 D3gQt4Gy6hiy7oRVXNVlUvnmRmmbI+yaD420JPrS3qDG79b6abqpPvEqRwp2+5DzW1ro 5aXg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=Cw3Q2nDkJ9Bv8PVsAGDCbfG6m5PVGNiCmox+HLaI3GE=; b=gU8ZcBKqj0pZAk+M8PVDcRYfySxscYUWN36Pqlje7xKqVJOHujDedmHqu32qX1h/B3 WNfPbfMMlVpiZv+tbbvJYm41oaGIKT2WqUMxaCKLsqduKw8wcTDOuuj+qZTqSqkWxMgJ IK+KSlg4+liPXiKQCSkV9pAwRk3+0MdIfQPkgwBYjiLM0trKyr0AihXEJr3TAvCGo2Tg uQpYSMSdTNhRHZOfNRZ0lhFiFenWcLD7J1FhXsjDlXg7jmCvPz2iRKYh7rdo5cna5rSK Y5Ow+OP3o+beQr6IFnB1nnVu9yW/ikBX/A7TRKH2dFeQIm8SCdXZ+I0kzHxtNRdqfQEY paVw== X-Gm-Message-State: AO0yUKUXPrGN1xZ5Lf6AMsAKPFbI6xNOwaEC43m9o/z+pbV6XjvsIo/N xisuBtBSHlu9kO7cT5BO1RhZtGdieoh3Z6j9Pb9PuA== X-Google-Smtp-Source: AK7set8PIsvFjgrQwufJLFQT67MNYzMOGV1d7JQLJAehTa092Ya4uJIbrwg+wDH31UncdizZSMR2WGwIMLPKliDCmY4= X-Received: by 2002:a1f:284e:0:b0:401:42f3:5659 with SMTP id o75-20020a1f284e000000b0040142f35659mr1000487vko.44.1676271654615; Sun, 12 Feb 2023 23:00:54 -0800 (PST) MIME-Version: 1.0 References: <20230208184203.2260394-1-elver@google.com> In-Reply-To: From: Marco Elver Date: Mon, 13 Feb 2023 08:00:00 +0100 Message-ID: Subject: Re: [PATCH -tip] kasan: Emit different calls for instrumentable memintrinsics To: Andrey Konovalov Cc: Peter Zijlstra , Masahiro Yamada , Nathan Chancellor , Nick Desaulniers , Nicolas Schier , Andrey Ryabinin , Alexander Potapenko , Dmitry Vyukov , Vincenzo Frascino , linux-kbuild@vger.kernel.org, kasan-dev@googlegroups.com, linux-kernel@vger.kernel.org, Ingo Molnar , Tony Lindgren , Ulf Hansson , linux-toolchains@vger.kernel.org, Mark Rutland Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 10 Feb 2023 at 22:37, Andrey Konovalov wrote: > > On Fri, Feb 10, 2023 at 7:41 PM Marco Elver wrote: > > > > On Fri, 10 Feb 2023 at 17:13, Andrey Konovalov wrote: > > [...] > > > > Probably the same should be done for SW_TAGS, because arm64 will be > > > > GENERIC_ENTRY at one point or another as well. > > > > > > Yes, makes sense. I'll file a bug for this once I fully understand the > > > consequences of these changes. > > > > > > > KASAN + GCC on x86 will have no mem*() instrumentation after > > > > 69d4c0d32186, which is sad, so somebody ought to teach it the same > > > > param as above. > > > > > > Hm, with that patch we would have no KASAN checking within normal mem* > > > functions (not the ones embedded by the compiler) on GENERIC_ENTRY > > > arches even with Clang, right? > > > > Yes, that's the point - normal mem*() functions cannot be instrumented > > with GENERIC_ENTRY within noinstr functions, because the compiler > > sometimes decides to transform normal assignments into > > memcpy()/memset(). And if mem*() were instrumented (as it was before > > 69d4c0d32186), that'd break things for these architectures. > > > > But since most code is normally instrumented, with the right compiler > > support (which the patch here enables), we just turn mem*() in > > instrumented functions into __asan_mem*(), and get the instrumentation > > as before. 69d4c0d32186 already added those __asan functions. The fact > > that KASAN used to override mem*() is just the wrong choice in a world > > where compilers decide to inline or outline these. From an > > instrumentation point of view at the compiler level, we need to treat > > them like any other instrumentable instruction (loads, stores, > > atomics, etc.): transform each instrumentable instruction into > > something that does the right checks. Only then can we be sure that we > > don't accidentally instrument something that shouldn't be (noinstr > > functions), because instead of relying on the compiler, we forced > > instrumentation on every mem*(). > > I meant to ask whether the normal mem* calls from instrumented > functions will also be transformed to __asan_mem*() by the compiler. > But following the godbolt link you shared, I see that this is true. > > Thank you for the explanation! > > So the overall negative impact of these changes is that we don't get > KASAN checking in both normal mem* calls and the ones formed by > transforming assignments for GENERIC_ENTRY architectures with GCC and > with older Clang. This is not great. I wonder if we then need to print > some kind of warning when the kernel is built with these compilers. Since these changes are already in -tip, and by judging from [1], there really is no other way. As-is, KASAN on x86 is already broken per [1] (though we got lucky thus far). Printing a warning wouldn't hurt, but I think nobody would notice the warning, and if somebody notices, they wouldn't care. Sooner or later, we just need to make sure that test robots (syzbot, etc.) have new compilers. > If these changes move forward, AFAIU, we can also drop these custom > mem* definitions for non-instrumented files for x86: > > https://elixir.bootlin.com/linux/latest/source/arch/x86/include/asm/string_64.h#L88 Yes, I think so. [1] https://lore.kernel.org/all/20230112194314.845371875@infradead.org/ Last but not least are you ok with this patch? This patch ought to be applied to the same tree as 69d4c0d32186 anyway, so this patch lives or dies by that change.