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=-3.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_ADSP_CUSTOM_MED,DKIM_SIGNED,DKIM_VALID,FSL_HELO_FAKE, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS 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 E3A6DC12002 for ; Wed, 21 Jul 2021 13:37:04 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id AFB8F60FEA for ; Wed, 21 Jul 2021 13:37:04 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org AFB8F60FEA 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-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=tT7jSGzUwlVYlqrOSQVms4JmER0PedC7YIY313q+jLY=; b=Vtu5lmJ+RsuOcc 2tMQxaLNnUe3D0Mk4kfZ7HKDF29MG9Rcg/u4pIf2Zp27QxlA1RHTBx1QpM8Gt311qJMOmvbCS3CcA 57hOlC8BR26q1/CS6yiZmkjjy1ctcGpxZTJWWYW0YupO209iUeKNcZR6e4t+5lphMmqhxbs9PeVgQ salT//8Xi+R8zHclT3ExlpViUKG918YBOW9QqQ9LBTm+nwo0yrgOjbxR25/1jHpgP466lVwZ/WXbx l8gC4BGXZjKI9IzbmiF7oXGqvycEaD4wapjyfxo4HoeRYGJpR0Yt2kaWyJ6JxKgeCq/n3fO7DcNm2 VewakLIw5gzwzYqA0BWQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1m6CNv-00FlTF-HQ; Wed, 21 Jul 2021 13:35:39 +0000 Received: from mail-wm1-x333.google.com ([2a00:1450:4864:20::333]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1m6CNs-00FlSg-8p for linux-arm-kernel@lists.infradead.org; Wed, 21 Jul 2021 13:35:38 +0000 Received: by mail-wm1-x333.google.com with SMTP id l6so1372089wmq.0 for ; Wed, 21 Jul 2021 06:35:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=THtvL95+Wdg5XKa0Q6wjHIENl0Y9TZHt3CwdcuBkgF8=; b=dYzzqSbTcHoAKX52adHhFah33UAvJMaxB1AIimYVp8/ShQNJyvzpQuyW4v9H9wQS7C 9oUflpJc11rRJeujwNJ/0OaIN7oOBEwVF7hMZpgJorzEtN81xw7UznEeWpFXOCMU+9Yf vvlCvBwEaXtKD+Mit2oA1qgc2x2cQOCvdQD8ad6Jmyx3IV+U1K1ht0zp7+FRoLComa9z 7nyg//dnCxl4r1LGd2OmqdUp+AIhctUAyBKbvsw/8+fgpsxlsGVuOTjFWbitTHU3+qua iXTTZxxsP4ssGpvdcORzQIsJquYp1mYs6zK7WvpK/a82z1I5x6mBKwGwDf4o5qiqrpRR CN8A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=THtvL95+Wdg5XKa0Q6wjHIENl0Y9TZHt3CwdcuBkgF8=; b=hdF3SJUMy62gV4rt70jZfS4K410+Bc+/CMLnnSvgJXEWRgHtF0vQdBJiCdPSeqjBnI nYC+jDWk2DjM27OPvVtXDyuNOoA0C9j8cHs41rjXQahBpKKlLxZoFvCzsIJoDD7yjreJ kIFeNaXcPU6MNMdQBcJLXGSffa0bafFM4jcjHIjvwogYJ+5FnfYVbENZcV1iJ5emFBnT ENMeukC9WH6xFsWusGzfMvxNIvxlYP3PHfTOEosEgjSMIKx13B5cG1RPULgHbveZv3/4 1jZDJpCW/oWHXi91v6fByN5FEYFz7PYdxcWkJi+EOMdEJa7veBMQt+VerHF9C+l52jnd rugA== X-Gm-Message-State: AOAM530GERQY8ecpdIcGIDFyDInAcvWPsZ2PPAMSsce6X9oNxpHlvgR0 VTfsRRTxt6EcuOHkp8ouP5/8xA== X-Google-Smtp-Source: ABdhPJyciOXfOFa9KQ+iYzD+QsJY74g96fGc86hUOZHCThYzhTYjYByChhSiI5DH0qDjIx7s8PRrdA== X-Received: by 2002:a1c:1bc3:: with SMTP id b186mr4197684wmb.27.1626874534544; Wed, 21 Jul 2021 06:35:34 -0700 (PDT) Received: from google.com ([2a00:79e0:d:210:efb1:2fcc:e84:52ad]) by smtp.gmail.com with ESMTPSA id e11sm33189602wrt.0.2021.07.21.06.35.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 21 Jul 2021 06:35:34 -0700 (PDT) Date: Wed, 21 Jul 2021 14:35:30 +0100 From: Quentin Perret To: Fuad Tabba Cc: maz@kernel.org, james.morse@arm.com, alexandru.elisei@arm.com, suzuki.poulose@arm.com, catalin.marinas@arm.com, will@kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, linux-kernel@vger.kernel.org, ardb@kernel.org, qwandor@google.com, dbrazdil@google.com, kernel-team@android.com Subject: Re: [PATCH 13/14] KVM: arm64: Restrict hyp stage-1 manipulation in protected mode Message-ID: References: <20210719104735.3681732-1-qperret@google.com> <20210719104735.3681732-14-qperret@google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210721_063536_379928_482CB94F X-CRM114-Status: GOOD ( 26.94 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Fuad, On Wednesday 21 Jul 2021 at 11:45:53 (+0100), Fuad Tabba wrote: > > +static int hyp_range_is_shared_walker(u64 addr, u64 end, u32 level, > > + kvm_pte_t *ptep, > > + enum kvm_pgtable_walk_flags flag, > > + void * const arg) > > +{ > > + enum kvm_pgtable_prot prot; > > + kvm_pte_t pte = *ptep; > > + > > + if (!kvm_pte_valid(pte)) > > + return -EPERM; > > + > > + prot = kvm_pgtable_hyp_pte_prot(pte); > > + if (!prot) > > + return -EPERM; > nit: is this check necessary? I guess not, the next one should do, I'll remove :) > > + /* Check that the page has been shared with the hypervisor before */ > > + if (prot != (PAGE_HYP | KVM_PGTABLE_STATE_SHARED | KVM_PGTABLE_STATE_BORROWED)) > > + return -EPERM; > > + > > + return 0; > > +} > > + > > +static int hyp_range_is_shared(phys_addr_t start, phys_addr_t end) > > +{ > > + struct kvm_pgtable_walker walker = { > > + .cb = hyp_range_is_shared_walker, > > + .flags = KVM_PGTABLE_WALK_LEAF, > > + }; > > + > > + return kvm_pgtable_walk(&pkvm_pgtable, (u64)__hyp_va(start), > > + end - start, &walker); > > +} > > + > > +static int check_host_share_hyp_walker(u64 addr, u64 end, u32 level, > > nit: It seems the convention is usually addr,size or start,end. Here > you're using addr,end. Well for some reason all the walkers in pgtable.c follow the addr,end convention, so I followed that. But in fact, as per [1] I might actually get rid of this walker in v2, so hopefully that'll make the issue go away. [1] https://lore.kernel.org/kvmarm/YPbwmVk1YD9+y7tr@google.com/ > > + kvm_pte_t *ptep, > > + enum kvm_pgtable_walk_flags flag, > > + void * const arg) > > +{ > > + enum kvm_pgtable_prot prot; > > + kvm_pte_t pte = *ptep; > > + > > + /* If invalid, only allow to share pristine pages */ > > + if (!kvm_pte_valid(pte)) > > + return pte ? -EPERM : 0; > > + > > + prot = kvm_pgtable_stage2_pte_prot(pte); > > + if (!prot) > > + return -EPERM; > > + > > + /* Cannot share a page that is not owned */ > > + if (prot & KVM_PGTABLE_STATE_BORROWED) > > + return -EPERM; > > + > > + /* Cannot share a page with restricted access */ > > + if ((prot & KVM_PGTABLE_PROT_RWX) ^ KVM_PGTABLE_PROT_RWX) > nit: isn't this clearer as > > if ((prot & KVM_PGTABLE_PROT_RWX) != KVM_PGTABLE_PROT_RWX) I guess it would be, I'll fix it up. > > + return -EPERM; > > + > > + /* Allow double-sharing (requires cross-checking the hyp stage-1) */ > > + if (prot & KVM_PGTABLE_STATE_SHARED) > > + return hyp_range_is_shared(addr, addr + 1); > > Why addr+1, rather than end? Because 'end' here is the 'end' that was passed to kvm_pgtable_walk() IIRC. What I want to do here is check if the page I'm currently visiting is already shared and if so, that it is shared with the hypervisor. But it's possible that only one page in the range of pages passed to __pkvm_host_share_hyp is already shared, so I need to check them one by one. Anyways, as per the discussion with Marc on [2], I'll probably switch to only accept sharing one page at a time, so all these issues should just go away as well! [2] https://lore.kernel.org/kvmarm/YPa6BGuUFjw8do+o@google.com/ > > + > > + return 0; > > +} > > + > > +static int check_host_share_hyp(phys_addr_t start, phys_addr_t end) > > +{ > > + struct kvm_pgtable_walker walker = { > > + .cb = check_host_share_hyp_walker, > > + .flags = KVM_PGTABLE_WALK_LEAF, > > + }; > > + > > + return kvm_pgtable_walk(&host_kvm.pgt, start, end - start, &walker); > > +} > > + > > +int __pkvm_host_share_hyp(phys_addr_t start, phys_addr_t end) > > +{ > > + enum kvm_pgtable_prot prot; > > + int ret; > > + > > + if (!range_is_memory(start, end)) > > + return -EINVAL; > > + > > + hyp_spin_lock(&host_kvm.lock); > > + hyp_spin_lock(&pkvm_pgd_lock); > > + > > + ret = check_host_share_hyp(start, end); > > + if (ret) > > + goto unlock; > > + > > + prot = KVM_PGTABLE_PROT_RWX | KVM_PGTABLE_STATE_SHARED; > > + ret = host_stage2_idmap_locked(start, end, prot); > > Just for me to understand this better. The id mapping here, which > wasn't taking place before this patch, is for tracking, right? Yes, exactly, I want to make sure to mark the page as shared (and borrowed) in the relevant page-tables to not forget about it :) Cheers, Quentin _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel