Received: by 2002:a05:7412:5112:b0:fa:6e18:a558 with SMTP id fm18csp362941rdb; Tue, 23 Jan 2024 01:51:34 -0800 (PST) X-Google-Smtp-Source: AGHT+IF467hKajh+k0SNxhYiPgPAFBoTvZTC8tCsw5ayp2tLNQuKLm6rvOzMTIDd/YbRnbP2eNER X-Received: by 2002:a05:6808:170b:b0:3bd:9761:a6e8 with SMTP id bc11-20020a056808170b00b003bd9761a6e8mr3001910oib.67.1706003494113; Tue, 23 Jan 2024 01:51:34 -0800 (PST) ARC-Seal: i=2; a=rsa-sha256; t=1706003494; cv=pass; d=google.com; s=arc-20160816; b=SybyFoqayUGfNJcuJI9W045Y/Cyn0tGhDKj3ZrfXlz9L4CC8W5LmbHfDcaPL3up+IA +c63cYnOGAadok9i/DtVJQEaUNi2fQsUzbqSYHRMuv99Xnyr+DQb244/Q5QIsATPkdh1 2Bzh+Lps7bq5SjMZx2I5o+qNgHd1Qo5fHIFvmpDE9VPrNYR2zgAUBOOV4NfhZ1/bNbJT L5SCq/YBGX/fonelEXaYur9myMbXt07BC0b8P4PYewfrPVPVacX0ncPXycNh6WLcST6I qN/BejOKwRnvbGzN6tKdAyhkqXY5i5mmXfKc2OUN70J2heY+uXIDChrFXDwG5jqR0KOm zWRg== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=in-reply-to:content-disposition:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:references:message-id:subject:cc :to:from:date; bh=2IgkmubL4hlV26BoX7FjQY4O94NhD2cnMDMOBIzGPVg=; fh=Rd15O7krEC/RG53mfcaeV6K3x6NRtpaeJ61dDXazl4E=; b=S4T3uo6aCR7LcTj0SNrt/MfDndGYW8eTGVl8qLAyAMLNXCs9YAVf3G2xHJjWiyOKcP g4jelkQy3vQ05PT+YWobLxxOS+1RYlAHx7pXOeFbDDnHkaJONJ3y4jc/+Uv2QW9jd9hJ Om8y6D8lUCGAUPykYiSND03z+kScKRGEIINpw0q8ioU++3dOyYjG2QzfW/H2MCF296qU aOvYjkYrlyY9mewy1jQuS3DO8eqCk69xuArwgxRCXjuhxuEmHc43+tLgvqqRcsD0nVsw 51dutdwiymwUVEGEr9oAtm2v3RYZNs3fqpiCWL9RuFlEws8r8zfIaD50XDEx6gyCzGsZ 89yQ== ARC-Authentication-Results: i=2; mx.google.com; arc=pass (i=1 spf=pass spfdomain=arm.com dmarc=pass fromdomain=arm.com); spf=pass (google.com: domain of linux-kernel+bounces-35035-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-35035-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=arm.com Return-Path: Received: from sv.mirrors.kernel.org (sv.mirrors.kernel.org. [2604:1380:45e3:2400::1]) by mx.google.com with ESMTPS id l33-20020a635721000000b005c693ea6618si9644455pgb.523.2024.01.23.01.51.33 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 23 Jan 2024 01:51:34 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-35035-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1; Authentication-Results: mx.google.com; arc=pass (i=1 spf=pass spfdomain=arm.com dmarc=pass fromdomain=arm.com); spf=pass (google.com: domain of linux-kernel+bounces-35035-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-35035-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=arm.com Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sv.mirrors.kernel.org (Postfix) with ESMTPS id 0252228B7D0 for ; Tue, 23 Jan 2024 09:51:02 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 30EBC5D914; Tue, 23 Jan 2024 09:46:43 +0000 (UTC) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id AD2115D904; Tue, 23 Jan 2024 09:46:40 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.140.110.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706003202; cv=none; b=YKTn0yocqvW1Q01+RbK4NMO8SNCKOpNf9N9vyxKR+9Owwndkggh1VNO4JbK/pIfHw8PlTugqgOWgVvnp7/KT6OmqF+4nyCJnNjifQ2xvAGubU3k3po35vPH2ksYhOFWmyhYdt6qjxdOQWmSyckOsJHcR2ki2EpVZZSqRwpJP3p8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706003202; c=relaxed/simple; bh=Xz5yCHNx8CJHFlmKFK8Vb+xsWX06h3/wC41N3TUgGoY=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=tEMSABVnUFocAb35VOk1RpchEE/7l8klhZTR4KOtEL4TdvED+7yBszJTqN19cZum1AtaVEEJvApYrdw4lhopREmM6mrWZRHY6a8hzRWwrerL2Kk1X0f1O7HE8IN4cYgcO68O86NhjwZ8loHVJbVtaWd07hZw1Q/oqQMFZZqMRUI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com; spf=pass smtp.mailfrom=arm.com; arc=none smtp.client-ip=217.140.110.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 531941FB; Tue, 23 Jan 2024 01:47:25 -0800 (PST) Received: from FVFF77S0Q05N (unknown [10.57.48.47]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id CDB753F5A1; Tue, 23 Jan 2024 01:46:38 -0800 (PST) Date: Tue, 23 Jan 2024 09:46:35 +0000 From: Mark Rutland To: Kees Cook Cc: linux-hardening@vger.kernel.org, "Gustavo A. R. Silva" , Bill Wendling , Justin Stitt , linux-kernel@vger.kernel.org Subject: Re: [PATCH 00/82] overflow: Refactor open-coded arithmetic wrap-around Message-ID: References: <20240122235208.work.748-kees@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240122235208.work.748-kees@kernel.org> On Mon, Jan 22, 2024 at 04:26:35PM -0800, Kees Cook wrote: > Hi, Hi Kees, > In our continuing effort to eliminate root causes of flaws in the kernel, > this series is the start to providing a way to have sensible coverage > for catching unexpected arithmetic wrap-around. > > A quick word on language: while discussing[1] the finer details of > the C standard's view on arithmetic, I was disabused of using the term > "overflow" when what I really mean is "wrap-around". When describing > security vulnerabilities, "overflow" is the common term and often used > interchangeably with "wrap-around". Strictly speaking, though, "overflow" > applies only to signed[2] and pointer[3] types, and "wrap-around" is for > unsigned[4]. An arithmetic "overflow" is considered undefined behavior, > which has caused our builds pain in the past, since "impossible" > conditions might get elided by the compiler. As a result, we build > with -fno-strict-overflow which coverts all "overflow" conditions into > "wrap-around" (i.e. 2s complement), regardless of type. > > All this is to say I am discussing arithmetic wrap-around, which is > the condition where the value exceeds a type's maximum value (or goes > below its minimum value) and wraps around. I'm not interested in the > narrow definition of "undefined behavior" -- we need to stamp out the > _unexpected_ behavior, where the kernel operates on a pathological value > that wrapped around without the code author's intent. With that in mind, I note that this patch primarily modifies addition operations, but leaves subtraction operations unchanged (even though those permit the value to go below the minimum, or above the maximum if a negative value is used as the subtrahend). Shouldn't we address both at the same time? I'll note that in many places the same logic is used for both the add and sub, and can legitimately overflow or underflow; I hope that whatever we use to suppress overflow warnings also ignores underflow. [...] Looking at the diffstat, I think you've missed a few places: > Documentation/process/deprecated.rst | 36 ++++++++ > arch/arc/kernel/unwind.c | 7 +- > arch/arm/nwfpe/softfloat.c | 2 +- > arch/arm64/include/asm/atomic_lse.h | 8 +- > arch/arm64/include/asm/stacktrace/common.h | 2 +- > arch/arm64/kvm/vgic/vgic-kvm-device.c | 6 +- > arch/arm64/kvm/vgic/vgic-mmio-v3.c | 2 +- > arch/arm64/kvm/vgic/vgic-v2.c | 10 ++- > arch/m68k/kernel/sys_m68k.c | 5 +- > arch/nios2/kernel/sys_nios2.c | 2 +- > arch/powerpc/platforms/powernv/opal-prd.c | 2 +- > arch/powerpc/xmon/xmon.c | 2 +- > arch/s390/include/asm/stacktrace.h | 6 +- > arch/s390/kernel/machine_kexec_file.c | 5 +- > arch/s390/mm/gmap.c | 4 +- > arch/s390/mm/vmem.c | 2 +- > arch/sh/kernel/sys_sh.c | 2 +- > arch/x86/include/asm/atomic.h | 2 +- > arch/x86/kernel/cpu/sgx/ioctl.c | 6 +- > arch/x86/kvm/svm/sev.c | 5 +- > crypto/adiantum.c | 2 +- > drivers/acpi/custom_method.c | 2 +- > drivers/char/agp/generic.c | 2 +- > drivers/crypto/amcc/crypto4xx_alg.c | 2 +- > drivers/crypto/axis/artpec6_crypto.c | 2 +- > drivers/dma-buf/dma-buf.c | 7 +- > drivers/fpga/dfl.c | 5 +- > drivers/fsi/fsi-core.c | 6 +- > drivers/gpu/drm/i915/i915_vma.c | 2 +- > drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.c | 8 +- > drivers/gpu/drm/vc4/vc4_validate.c | 7 +- > drivers/md/dm-switch.c | 2 +- > drivers/md/dm-verity-target.c | 2 +- > drivers/md/dm-writecache.c | 2 +- > drivers/net/ethernet/sun/niu.c | 5 +- > drivers/net/wireless/ath/wil6210/wmi.c | 2 +- > drivers/net/wireless/marvell/mwifiex/pcie.c | 6 +- > drivers/net/xen-netback/hash.c | 2 +- > drivers/pci/pci.c | 2 +- > drivers/remoteproc/pru_rproc.c | 2 +- > drivers/remoteproc/remoteproc_elf_loader.c | 2 +- > drivers/remoteproc/remoteproc_virtio.c | 4 +- > drivers/scsi/mpt3sas/mpt3sas_ctl.c | 2 +- > drivers/scsi/sd_zbc.c | 2 +- > drivers/staging/vme_user/vme.c | 2 +- > drivers/vhost/vringh.c | 8 +- > drivers/virtio/virtio_pci_modern_dev.c | 4 +- > fs/aio.c | 2 +- > fs/bcachefs/bkey.c | 4 +- > fs/bcachefs/fs.c | 2 +- > fs/bcachefs/quota.c | 2 +- > fs/bcachefs/util.c | 2 +- > fs/btrfs/extent_map.c | 6 +- > fs/btrfs/extent_map.h | 6 +- > fs/btrfs/ordered-data.c | 2 +- > fs/ext4/block_validity.c | 2 +- > fs/ext4/extents.c | 5 +- > fs/ext4/resize.c | 2 +- > fs/f2fs/file.c | 2 +- > fs/f2fs/verity.c | 2 +- > fs/hpfs/alloc.c | 2 +- > fs/ntfs3/record.c | 4 +- > fs/ocfs2/resize.c | 2 +- > fs/read_write.c | 8 +- > fs/remap_range.c | 2 +- > fs/select.c | 13 +-- > fs/smb/client/readdir.c | 5 +- > fs/smb/client/smb2pdu.c | 4 +- > fs/udf/balloc.c | 4 +- > include/linux/compiler_types.h | 29 +++++- This misses the include/asm-generic/{atomic,atomic64}.h implementations. This also misses the include/linux/atomic/atomic-arch-fallback.h implementations. Those are generated from the scripts/atomic/fallbacks/* templates, and you'll need to adjust at least fetch_add_unless and inc_unless_negative. As noted on other patches, my preference is to use add_wrap() in those. > include/linux/overflow.h | 76 +++++++++++++++- > ipc/mqueue.c | 2 +- > ipc/shm.c | 6 +- > kernel/bpf/verifier.c | 12 +-- > kernel/time/timekeeping.c | 2 +- > lib/Kconfig.ubsan | 27 ++++++ This misses lib/atomic64.c. Thanks, Mark.