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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id AB327C46CD2 for ; Wed, 24 Jan 2024 16:16:23 +0000 (UTC) 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:Reply-To:List-Subscribe:List-Help: List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To:From:References: Cc:To:Subject:MIME-Version:Date:Message-ID:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=nRYcMxAFXCXJU/w+YWgKFU6XXLVJ/p4SmbDrmOmt02o=; b=L81kiG4eUk/fD8 qIFsKCM7ung/CtNDlxAs90SI1fzzF1Ews5bNTfOo9OpK0QsNn9k1/ixYmYzzV5JLcdN6rdKQZ5NNJ TZLTfhbNzCe2Z98UwWllVQRQ+gHDffBv3AAo0t04I9orYKR63hAtAspBIGdhD6diA37dAVmD9DwvG v7XNU5Cc1J5rBWDEfo9VTHzZlPvbY15UcL1iY7ydXRdrGMWsHtZEpjpoTJp0sKtYbuzYbSfdyUxBq jZUlfQd90sqnyG0YCZ9TZX84b+WjeTtMI4ZkowY0myKTWoLJepXu2D4vnbXxlZLLmvI3VrA65iDcK iGHdhfHigQMQWXwp9VMQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1rSfus-0048ur-1k; Wed, 24 Jan 2024 16:15:54 +0000 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1rSewv-003sjQ-2W for linux-arm-kernel@lists.infradead.org; Wed, 24 Jan 2024 15:13:59 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1706109235; h=from:from:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=AmpPYmHl13Io6EkojaUZhZmn9ypUW1G8T5NB4u+9OLg=; b=beX/zhwzjdHetkXSJy1kSSY+Etp934ta7hLyJASZOg9VaF0NhJI+ogoRd1DEv1VF2dzRkH HZFZxCKKklDxTo+enmDQPy3BKJ4tPxRkbEqtrKjFpeOC9eqYeUlomomF5lXGXYzyjBuGoe YoK7P+H2paXJHPjuOSLSkMMJLWJaZeY= Received: from mail-qv1-f71.google.com (mail-qv1-f71.google.com [209.85.219.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-522-j4hVvwXCNden0pbCwGj5Dg-1; Wed, 24 Jan 2024 10:13:53 -0500 X-MC-Unique: j4hVvwXCNden0pbCwGj5Dg-1 Received: by mail-qv1-f71.google.com with SMTP id 6a1803df08f44-68192812b38so51920046d6.0 for ; Wed, 24 Jan 2024 07:13:53 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1706109233; x=1706714033; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:reply-to:user-agent:mime-version:date :message-id:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=AmpPYmHl13Io6EkojaUZhZmn9ypUW1G8T5NB4u+9OLg=; b=lyHLVi0JatlMndVDBlI4JfkdrkKFx7LmwZ0+1jqjpMq75jFHvVzh85kjParcQCFivi XM4OnNgvd49fVfFA7XZBqgfgF8GFButlQ3Ba3cCLELL+Yd5UWzWcL3YgCb3S3h7P9bOD n3XBF4lmfWpzCLpsYCBplNhPqKdI9JjiB8YDgZBTuPXdr1EXby+5l+Y5UrcMCv0PCGdR w0+qmEUDaijyFwmrPaT51T6Y2NtFrli/BMK8PmxA7JL4/EqJxfaMQTZXEvJfuPUGffid c5boyjB06CtDqWTeWmtKSrG3+ys24ZDHIWnd5JNyuBAHIacsU/RuJORA7eTYa1GZ3DQ1 Uo5A== X-Gm-Message-State: AOJu0Yyo2kOikkZw4BCUMx7+hphWR4YIWTq/wwWHU4kJ1FiAzZOL2833 Y4Zc4xUjybdfvpGcgQS+ygof2paMY8foagmUJbcOCNwYEDCUcwK7EegnCsv7Xh9O9IPBtfFkMBZ TUMqFpGmqefO9Sm8LMZo35IScKICphhnF0/gOf5Vyf55ft5VlbmDkSwi2d0ElQiBrBoX0TSdl X-Received: by 2002:ad4:5bcc:0:b0:681:99b:ea0 with SMTP id t12-20020ad45bcc000000b00681099b0ea0mr2636946qvt.54.1706109233178; Wed, 24 Jan 2024 07:13:53 -0800 (PST) X-Google-Smtp-Source: AGHT+IHOrCqKRabSy51esnpYkStX9FYC3518F8eYJFX1zOlKRM3AgbDzTJsl87QnO68+09jDXkknnQ== X-Received: by 2002:ad4:5bcc:0:b0:681:99b:ea0 with SMTP id t12-20020ad45bcc000000b00681099b0ea0mr2636921qvt.54.1706109232837; Wed, 24 Jan 2024 07:13:52 -0800 (PST) Received: from ?IPV6:2a01:e0a:59e:9d80:527b:9dff:feef:3874? ([2a01:e0a:59e:9d80:527b:9dff:feef:3874]) by smtp.gmail.com with ESMTPSA id z15-20020a0cfc0f000000b0067f2559fd5esm4562012qvo.34.2024.01.24.07.13.48 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 24 Jan 2024 07:13:52 -0800 (PST) Message-ID: <86b9e255-9d19-44ee-b6c4-544507756183@redhat.com> Date: Wed, 24 Jan 2024 16:13:48 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 24/82] KVM: arm64: vgic: Refactor intentional wrap-around calculation To: Marc Zyngier , Kees Cook Cc: linux-hardening@vger.kernel.org, Oliver Upton , James Morse , Suzuki K Poulose , Zenghui Yu , Catalin Marinas , Will Deacon , Reiji Watanabe , Ricardo Koller , Raghavendra Rao Ananta , Quentin Perret , Jean-Philippe Brucker , linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, "Gustavo A. R. Silva" , Bill Wendling , Justin Stitt , linux-kernel@vger.kernel.org References: <20240122235208.work.748-kees@kernel.org> <20240123002814.1396804-24-keescook@chromium.org> <86o7dc8gu5.wl-maz@kernel.org> From: Eric Auger In-Reply-To: <86o7dc8gu5.wl-maz@kernel.org> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240124_071357_920111_16848384 X-CRM114-Status: GOOD ( 20.28 ) 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: , Reply-To: eric.auger@redhat.com 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 On 1/23/24 11:49, Marc Zyngier wrote: > On Tue, 23 Jan 2024 00:26:59 +0000, > Kees Cook wrote: >> In an effort to separate intentional arithmetic wrap-around from >> unexpected wrap-around, we need to refactor places that depend on this >> kind of math. One of the most common code patterns of this is: >> >> VAR + value < VAR >> >> Notably, this is considered "undefined behavior" for signed and pointer >> types, which the kernel works around by using the -fno-strict-overflow >> option in the build[1] (which used to just be -fwrapv). Regardless, we >> want to get the kernel source to the position where we can meaningfully >> instrument arithmetic wrap-around conditions and catch them when they >> are unexpected, regardless of whether they are signed[2], unsigned[3], >> or pointer[4] types. >> >> Refactor open-coded unsigned wrap-around addition test to use >> check_add_overflow(), retaining the result for later usage (which removes >> the redundant open-coded addition). This paves the way to enabling the >> wrap-around sanitizers in the future. >> >> Link: https://git.kernel.org/linus/68df3755e383e6fecf2354a67b08f92f18536594 [1] >> Link: https://github.com/KSPP/linux/issues/26 [2] >> Link: https://github.com/KSPP/linux/issues/27 [3] >> Link: https://github.com/KSPP/linux/issues/344 [4] >> Cc: Marc Zyngier >> Cc: Oliver Upton >> Cc: James Morse >> Cc: Suzuki K Poulose >> Cc: Zenghui Yu >> Cc: Catalin Marinas >> Cc: Will Deacon >> Cc: Reiji Watanabe >> Cc: Eric Auger >> Cc: Ricardo Koller >> Cc: Raghavendra Rao Ananta >> Cc: Quentin Perret >> Cc: Jean-Philippe Brucker >> Cc: linux-arm-kernel@lists.infradead.org >> Cc: kvmarm@lists.linux.dev >> Signed-off-by: Kees Cook >> --- >> arch/arm64/kvm/vgic/vgic-kvm-device.c | 6 ++++-- >> arch/arm64/kvm/vgic/vgic-v2.c | 10 ++++++---- >> 2 files changed, 10 insertions(+), 6 deletions(-) >> >> diff --git a/arch/arm64/kvm/vgic/vgic-kvm-device.c b/arch/arm64/kvm/vgic/vgic-kvm-device.c >> index f48b8dab8b3d..0eec5344d203 100644 >> --- a/arch/arm64/kvm/vgic/vgic-kvm-device.c >> +++ b/arch/arm64/kvm/vgic/vgic-kvm-device.c >> @@ -18,17 +18,19 @@ int vgic_check_iorange(struct kvm *kvm, phys_addr_t ioaddr, >> phys_addr_t addr, phys_addr_t alignment, >> phys_addr_t size) >> { >> + phys_addr_t sum; >> + >> if (!IS_VGIC_ADDR_UNDEF(ioaddr)) >> return -EEXIST; >> >> if (!IS_ALIGNED(addr, alignment) || !IS_ALIGNED(size, alignment)) >> return -EINVAL; >> >> - if (addr + size < addr) >> + if (check_add_overflow(addr, size, &sum)) >> return -EINVAL; >> >> if (addr & ~kvm_phys_mask(&kvm->arch.mmu) || >> - (addr + size) > kvm_phys_size(&kvm->arch.mmu)) >> + sum > kvm_phys_size(&kvm->arch.mmu)) > nit: 'sum' doesn't mean much in this context. Something like 'end' > would be much more descriptive. > >> return -E2BIG; >> >> return 0; >> diff --git a/arch/arm64/kvm/vgic/vgic-v2.c b/arch/arm64/kvm/vgic/vgic-v2.c >> index 7e9cdb78f7ce..c8d1e965d3b7 100644 >> --- a/arch/arm64/kvm/vgic/vgic-v2.c >> +++ b/arch/arm64/kvm/vgic/vgic-v2.c >> @@ -273,14 +273,16 @@ void vgic_v2_enable(struct kvm_vcpu *vcpu) >> /* check for overlapping regions and for regions crossing the end of memory */ >> static bool vgic_v2_check_base(gpa_t dist_base, gpa_t cpu_base) >> { >> - if (dist_base + KVM_VGIC_V2_DIST_SIZE < dist_base) >> + gpa_t dist_sum, cpu_sum; > Same here: dist_end, cpu_end. I do agree. > >> + >> + if (check_add_overflow(dist_base, KVM_VGIC_V2_DIST_SIZE, &dist_sum)) >> return false; >> - if (cpu_base + KVM_VGIC_V2_CPU_SIZE < cpu_base) >> + if (check_add_overflow(cpu_base, KVM_VGIC_V2_CPU_SIZE, &cpu_sum)) >> return false; >> >> - if (dist_base + KVM_VGIC_V2_DIST_SIZE <= cpu_base) >> + if (dist_sum <= cpu_base) >> return true; >> - if (cpu_base + KVM_VGIC_V2_CPU_SIZE <= dist_base) >> + if (cpu_sum <= dist_base) >> return true; >> >> return false; > With these nits addressed, and assuming you intend to merge the whole > series yourself: > > Acked-by: Marc Zyngier assuming above suggested changes, Reviewed-by: Eric Auger Eric > > M. > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel