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=-8.4 required=3.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=ham 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 D94C9ECE562 for ; Wed, 19 Sep 2018 11:54:27 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 6FA76214C2 for ; Wed, 19 Sep 2018 11:54:27 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="PYuijPmD" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6FA76214C2 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731378AbeISRcA (ORCPT ); Wed, 19 Sep 2018 13:32:00 -0400 Received: from mail-io1-f66.google.com ([209.85.166.66]:40470 "EHLO mail-io1-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731338AbeISRcA (ORCPT ); Wed, 19 Sep 2018 13:32:00 -0400 Received: by mail-io1-f66.google.com with SMTP id l14-v6so4213013iob.7 for ; Wed, 19 Sep 2018 04:54:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=TN3QNhjkYmw+HaBkvgCjeas8QL9MY9/JuWC0ExTNN4o=; b=PYuijPmDpDUM5Hb3P+jRogRYdga0+2EzCa9Sqq5z9BIgrwUVwZQ8dE5LQTQPaMqz+a 8fiBxh7PtngyuI7n2nluQovcivuvczjAGFAyKowM2V0nuCs96jqo1hsYwylJ61PXBZGV bbhragISEp7Ye4Z2oFx5DSeEmA55fWNhc2JTmduIMwzBNW00bSDvDwsiYd0Z0vnHFflL k4YNy9Gsz5GO//BYNRshaLumRIG9Y/oy5PsuGsur2yqdHCbv7YZwKkJg0i5VWfT2kWXE ymoWZnSlqOH9zPPjE7MyezNleyYOEGlcNseQJkQz1tW7fIR2jZvk+JNp7ghEsVeeTE29 AeZg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=TN3QNhjkYmw+HaBkvgCjeas8QL9MY9/JuWC0ExTNN4o=; b=JG6nsEHIMBiJcQz7XwtfU+r2fX988vx7AuGhJsNUuiV4l/xl+UGXy/ljOFVQDSTVKu iKGiJc9Z2lKqdPYAunA7S7VpvVq6V2SMmAXXxVwAbQzMY3kdUtE+hSCdCIQRJOasYI8I 5jlNh53czpg3v2RXmQenGlaC/e+EfgE/rFidL6+8flD8Rq+VztX+VnGDzTva8j0fiFf7 ba6Z699MqSdxUDpN7FOFBV/WHC6Yt8vzHzzxJH7IjT+wbtGNm11JAt43x00xv0wze5oz gmfhNW/1sb4YMmR+N+JINBSTUsA7f1h/4YTOT/voqtcGpRk1QgVzECtQ9WQNT4eIIQsP f0iA== X-Gm-Message-State: APzg51BHaC6ERrjczEQKmVSNQyVn+40xvkmG2hjr41LWq2S3ji7hoeOI kVKDh2/BxaSa8Oe+EZexKCwapqwuVtkdJDB3chbf+g== X-Google-Smtp-Source: ANB0Vda/TSY5GBQv89JSajwxw8ANC9OEuaIBS3CoGpgBvPLzzhDlrCxmenhqlK89cazbWen0QpQ4VlXoG+pv5XijtSE= X-Received: by 2002:a6b:1707:: with SMTP id 7-v6mr28516531iox.119.1537358064356; Wed, 19 Sep 2018 04:54:24 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a02:c54e:0:0:0:0:0 with HTTP; Wed, 19 Sep 2018 04:54:23 -0700 (PDT) In-Reply-To: References: <4267d0903e0fdf9c261b91cf8a2bf0f71047a43c.1535462971.git.andreyknvl@google.com> From: Andrey Konovalov Date: Wed, 19 Sep 2018 13:54:23 +0200 Message-ID: Subject: Re: [PATCH v6 14/18] khwasan: add hooks implementation To: Dmitry Vyukov Cc: Andrey Ryabinin , Alexander Potapenko , Catalin Marinas , Will Deacon , Christoph Lameter , Andrew Morton , Mark Rutland , Nick Desaulniers , Marc Zyngier , Dave Martin , Ard Biesheuvel , "Eric W . Biederman" , Ingo Molnar , Paul Lawrence , Geert Uytterhoeven , Arnd Bergmann , "Kirill A . Shutemov" , Greg Kroah-Hartman , Kate Stewart , Mike Rapoport , kasan-dev , "open list:DOCUMENTATION" , LKML , Linux ARM , linux-sparse@vger.kernel.org, Linux-MM , "open list:KERNEL BUILD + fi..." , Kostya Serebryany , Evgeniy Stepanov , Lee Smith , Ramana Radhakrishnan , Jacob Bramley , Ruben Ayrapetyan , Jann Horn , Mark Brand , Chintan Pandya , Vishwath Mohan Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Sep 12, 2018 at 8:30 PM, Dmitry Vyukov wrote: > On Wed, Aug 29, 2018 at 1:35 PM, Andrey Konovalov wrote: >> void kasan_unpoison_shadow(const void *address, size_t size) >> { >> - kasan_poison_shadow(address, size, 0); >> + u8 tag = get_tag(address); >> + >> + /* Perform shadow offset calculation based on untagged address */ > > The comment is not super-useful. It would be more useful to say why we > need to do this. > Most callers explicitly untag pointer passed to this function, for > some it's unclear if the pointer contains tag or not. > For example, __hwasan_tag_memory -- what does it accept? Tagged or untagged? There are some callers that pass tagged pointers to this functions, e.g. ksize or kasan_unpoison_object_data. I'll expand the comment. > > >> + address = reset_tag(address); >> + >> + kasan_poison_shadow(address, size, tag); >> >> if (size & KASAN_SHADOW_MASK) { >> u8 *shadow = (u8 *)kasan_mem_to_shadow(address + size); >> - *shadow = size & KASAN_SHADOW_MASK; >> + >> + if (IS_ENABLED(CONFIG_KASAN_HW)) >> + *shadow = tag; >> + else >> + *shadow = size & KASAN_SHADOW_MASK; >> } >> } > > > It seems that this function is just different for kasan and khwasan. > Currently for kasan we have: > > kasan_poison_shadow(address, size, tag); > if (size & KASAN_SHADOW_MASK) { > u8 *shadow = (u8 *)kasan_mem_to_shadow(address + size); > *shadow = size & KASAN_SHADOW_MASK; > } > > But what we want to say for khwasan is: > > kasan_poison_shadow(address, round_up(size, KASAN_SHADOW_SCALE_SIZE), > get_tag(address)); > > Not sure if we want to keep a common implementation or just have > separate implementations... As per offline discussion leaving as is. >> void kasan_free_pages(struct page *page, unsigned int order) >> @@ -235,6 +248,7 @@ void kasan_cache_create(struct kmem_cache *cache, unsigned int *size, >> slab_flags_t *flags) >> { >> unsigned int orig_size = *size; >> + unsigned int redzone_size = 0; > > This variable seems to be always initialized below. We don't general > initialize local variables in this case. Will fix in v7. >> void check_memory_region(unsigned long addr, size_t size, bool write, >> unsigned long ret_ip) >> { >> + u8 tag; >> + u8 *shadow_first, *shadow_last, *shadow; >> + void *untagged_addr; >> + >> + tag = get_tag((const void *)addr); >> + >> + /* Ignore accesses for pointers tagged with 0xff (native kernel > > /* on a separate line Will fix in v7. > >> + * pointer tag) to suppress false positives caused by kmap. >> + * >> + * Some kernel code was written to account for archs that don't keep >> + * high memory mapped all the time, but rather map and unmap particular >> + * pages when needed. Instead of storing a pointer to the kernel memory, >> + * this code saves the address of the page structure and offset within >> + * that page for later use. Those pages are then mapped and unmapped >> + * with kmap/kunmap when necessary and virt_to_page is used to get the >> + * virtual address of the page. For arm64 (that keeps the high memory >> + * mapped all the time), kmap is turned into a page_address call. >> + >> + * The issue is that with use of the page_address + virt_to_page >> + * sequence the top byte value of the original pointer gets lost (gets >> + * set to KHWASAN_TAG_KERNEL (0xFF). > > Missed closing bracket. Will fix in v7.