From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 29DB4C433DB for ; Tue, 12 Jan 2021 21:16:53 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 8E59223122 for ; Tue, 12 Jan 2021 21:16:52 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 8E59223122 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 1329B6B00E2; Tue, 12 Jan 2021 16:16:52 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 0E45B6B00E4; Tue, 12 Jan 2021 16:16:52 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id F15D56B00E5; Tue, 12 Jan 2021 16:16:51 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0184.hostedemail.com [216.40.44.184]) by kanga.kvack.org (Postfix) with ESMTP id D87286B00E2 for ; Tue, 12 Jan 2021 16:16:51 -0500 (EST) Received: from smtpin12.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 94D3E181AEF2A for ; Tue, 12 Jan 2021 21:16:51 +0000 (UTC) X-FDA: 77698382622.12.actor57_3d0f15127518 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin12.hostedemail.com (Postfix) with ESMTP id 74E4C18055A08 for ; Tue, 12 Jan 2021 21:16:51 +0000 (UTC) X-HE-Tag: actor57_3d0f15127518 X-Filterd-Recvd-Size: 5872 Received: from mail-pj1-f41.google.com (mail-pj1-f41.google.com [209.85.216.41]) by imf33.hostedemail.com (Postfix) with ESMTP for ; Tue, 12 Jan 2021 21:16:50 +0000 (UTC) Received: by mail-pj1-f41.google.com with SMTP id iq13so2421180pjb.3 for ; Tue, 12 Jan 2021 13:16:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=1LiS/Ah7gWBCj+VqG26e57TUr+FTn3EftEs4Z008F6g=; b=Tte5vz34Ez295H9ux5RiqtzRXjKmLGKaImkiavOzuENuH8a2CsP3SmS92MtBal8eUp sYDUR+bThu/Q5yWFGkJvZ2OKhjN5KcIGfqgtA6GQl9braYM0IWn3jsvE/mLYMC1o03ge h6rq9fDZshnS8HfvQ6HP7G9TV6SXofpF0/1RWFQMsyIxwDl+ajLsVOIXj3/Z57NghYDd Dn6ca1dOeV51P6owuT+ORL/XRhoQJL1KJiQdjtLklX+tlJCBA3hXi8a9a/rP06MEVe6C TMTkhQ2IvCJYfLX6Y/4ZDCPSsECnId8AVHQ2pibJAbnDZOtBGQ+MNrgRKDQaVY65qwsT u1Ew== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=1LiS/Ah7gWBCj+VqG26e57TUr+FTn3EftEs4Z008F6g=; b=tp7k/tYNbTalk5zyWrpZywzqbrrE0LN8At+Ozelf39DHUdgdR9JPnVDy4ieCMCdSmE 2yzglRiqkJoossh9RFRsSCJnH49uJppPCmR9VgC9cmoRSaqAKE7OgdrmhaQbe2x13t10 Z6o09IXE3topHqdYEB5J3YPFhcB9PzLQ996HplrcGLq5rdSFIRAvB0r6UbyqCyyzhPV5 eLsesi0+eZP9dgTPvFhHvXHe0du2opfmR9KZ7YWwYmTLYORoS1KmEFsB4jPu90u37Vbq mWHNIPcDrttF8GG2w2+MS2gpMGAMkjkaBBifLRD6L/sP8BtAPOpPkS+J4YnQeAiwisHK pIIA== X-Gm-Message-State: AOAM5317GaixGjYghTzFhKt8Yrl3SaWOg1xwOFciszwa1tnKKjoC3S2C CnQCSs2+1GUu/59GHLiuz/XR1hT97Pq5j74Yl0/Qdg== X-Google-Smtp-Source: ABdhPJwg8P4BII9dX3v26wdnHYhmLKAAAuqTIhWoAo7A3cJC9jHhaKbbI8B4c56F3QMnTil996ruTXdtanNOKjipg9w= X-Received: by 2002:a17:90b:1087:: with SMTP id gj7mr1070271pjb.41.1610486209574; Tue, 12 Jan 2021 13:16:49 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: Andrey Konovalov Date: Tue, 12 Jan 2021 22:16:38 +0100 Message-ID: Subject: Re: [PATCH 10/11] kasan: fix bug detection via ksize for HW_TAGS mode To: Marco Elver Cc: Catalin Marinas , Vincenzo Frascino , Dmitry Vyukov , Alexander Potapenko , Andrew Morton , Will Deacon , Andrey Ryabinin , Evgenii Stepanov , Branislav Rankov , Kevin Brodsky , kasan-dev , Linux ARM , Linux Memory Management List , LKML Content-Type: text/plain; charset="UTF-8" X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue, Jan 12, 2021 at 3:32 PM Marco Elver wrote: > > > +/* > > + * Unlike kasan_check_read/write(), kasan_check_byte() is performed even for > > + * the hardware tag-based mode that doesn't rely on compiler instrumentation. > > + */ > > We have too many check-functions, and the name needs to be more precise. > Intuitively, I would have thought this should have access-type, i.e. > read or write, effectively mirroring a normal access. > > Would kasan_check_byte_read() be better (and just not have a 'write' > variant because we do not need it)? This would restore ksize() closest > to what it was before (assuming reporting behaviour is fixed, too). > > void kasan_poison(const void *address, size_t size, u8 value); > > void kasan_unpoison(const void *address, size_t size); > > -bool kasan_check_invalid_free(void *addr); > > +bool kasan_check(const void *addr); > > Definitely prefer shorted names, but we're in the unfortunate situation > of having numerous kasan_check-functions, so we probably need to be more > precise. > > kasan_check() makes me think this also does reporting, but it does not > (it seems to only check the metadata for validity). > > The internal function could therefore be kasan_check_allocated() (it's > now the inverse of kasan_check_invalid_free()). Re: kasan_check_byte(): I think the _read suffix is only making the name longer. ksize() isn't checking that the memory is readable (or writable), it's checking that it's addressable. At least that's the intention of the annotation, so it makes sense to name it correspondingly despite the implementation. Having all kasan_check_*() functions both checking and reporting makes sense, so let's keep the kasan_check_ prefix. What isn't obvious from the name is that this function is present for every kasan mode. Maybe kasan_check_byte_always()? Although it also seems too long. But I'm OK with keeping kasan_check_byte(). Re kasan_check(): Here we can use Andrew's suggestion about the name being related to what the function returns. And also drop the kasan_check_ prefix as this function only does the checking. Let's use kasan_byte_accessible() instead of kasan_check(). > > +bool __kasan_check_byte(const void *address, unsigned long ip) > > +{ > > + if (!kasan_check(address)) { > > + kasan_report_invalid_free((void *)address, ip); > > This is strange: why does it report an invalid free? Should this be a > use-after-free? I think this could just call kasan_report(....) for 1 > byte, and we'd get the right report. Will fix in v2. Thanks!