From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f174.google.com (mail-pl1-f174.google.com [209.85.214.174]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id E6E3D393 for ; Wed, 8 May 2024 22:54:37 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.174 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1715208879; cv=none; b=a3aMJc0BgVze25mZaWdT64gsfTLBwFXII2bT2uBm5gaDRmj8ICDukTirdOjQg5HEQ/nHMcI5BYjot7qtP2E8j747W4/vdrWhew8OgbXvn14S3JdhCwsqD5l2CoAPeOWRqj2KLmmsFW5EqnpIcXRJYayVnPfbPAH1d+iAZIkDORw= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1715208879; c=relaxed/simple; bh=gyQ55hQgJIIwlukOBDSk95pxDialupoX6yg1FkBruGA=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=NVbjQTOUchu7aU3UahkkGAs0d4FODQaZQUyckf1i/zFFaSw5eg85whUCw+ZeGqx/PTHfVQ9fxAIEWufA70eZNQK7tjTt80VBJ9bDAeDaf8F+BuFj4wVOIeORaFi70zrE5JPqcXOpZaEnfPA4Uj/55i3vSFTVHSUByAZgWgfJ/bs= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=chromium.org; spf=pass smtp.mailfrom=chromium.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b=TzaDaclW; arc=none smtp.client-ip=209.85.214.174 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=chromium.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="TzaDaclW" Received: by mail-pl1-f174.google.com with SMTP id d9443c01a7336-1ec41d82b8bso1808835ad.2 for ; Wed, 08 May 2024 15:54:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1715208877; x=1715813677; darn=lists.linux.dev; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=3u1Pl3pGa/v+VPDPwQjOTFukEhJKGI8Aryv6GP6qkbs=; b=TzaDaclWs0Cf2wA5+KxDIt6caIrVVX0ws8NYDxqUuD0J9It8IxgQI6+ybvrc6psmoX 1A2vODWKbqNOo6pT/UzCy16Yu2xO71EwTOwwDx8M42f0p7UNQQs71uvmqA+VypKKVUs7 5bHGLlGtp1aHZ02yOC4TLcXn7Czh35M4s65A0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1715208877; x=1715813677; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=3u1Pl3pGa/v+VPDPwQjOTFukEhJKGI8Aryv6GP6qkbs=; b=UX9B3WmQAHyl9ZlOTHCpTHa+yH4IxP3qIW63PtIWZCklE/WSkR9o8+ZmluMc0Yj2C5 kLawfMNGsV9lkFDZez4TayXTZu13BmX9UoTofuUm160UPDVSdb/RVkDRN6KOPXwdp1u5 i8u4o249tzr0RpoHk1WjBxUgcp8nJ6eyvPYk6+66O4T/ZGVw7zc2KnmpGk4Zk7PvkxbB bWBty++PqAEkFOCfwidN1o8CefW5vOEhpsVXUUBIWUWK7pC1EjLfEmz3DrO9Evn0wX8D evviNrDS1PtxSC3OP0kfaRgDI5kuLp6UQ/ZJ56cruz0N5k1iPDQehohkzi2OF+ZOnUvK n/aA== X-Forwarded-Encrypted: i=1; AJvYcCV/JvxjjapBEE/Y+RkW83de0nNVYoX+xnBT2GhrfD51FJDI5kJRC3/6Fa9Kgq6YsUMmY6xEUA/tIiSMZFvznl9T7nCoQg== X-Gm-Message-State: AOJu0YzbxG64AK+YaDSAJTQTECLAZ3JnT90wx5uCYv6ywkQTjeBPEol4 bC6v6FP+4kpOePy9TpeTe/Y3ASaZQKq3k6TmgcXaEe2YPWHDiAr2AQDVMmnbX8a/K2buzfP4hbY = X-Google-Smtp-Source: AGHT+IF5qLS8MIZu8bkrdkM2TSnYV54A1dmV9tmHQoSYXNzqgbagdmzZsQoufeCX4B3wWJs7UZAp1w== X-Received: by 2002:a17:902:c947:b0:1e8:4ad9:cbdf with SMTP id d9443c01a7336-1eeb0187168mr50075185ad.13.1715208877180; Wed, 08 May 2024 15:54:37 -0700 (PDT) Received: from www.outflux.net ([198.0.35.241]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-1ef0b9d1899sm714405ad.21.2024.05.08.15.54.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 08 May 2024 15:54:36 -0700 (PDT) Date: Wed, 8 May 2024 15:54:36 -0700 From: Kees Cook To: Linus Torvalds Cc: Justin Stitt , Peter Zijlstra , Mark Rutland , linux-hardening@vger.kernel.org, linux-kernel@vger.kernel.org, llvm@lists.linux.dev Subject: Re: [RFC] Mitigating unexpected arithmetic overflow Message-ID: <202405081354.B0A8194B3C@keescook> References: <202404291502.612E0A10@keescook> <202405081144.D5FCC44A@keescook> Precedence: bulk X-Mailing-List: llvm@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: On Wed, May 08, 2024 at 01:07:38PM -0700, Linus Torvalds wrote: > On Wed, 8 May 2024 at 12:45, Kees Cook wrote: > > > > On Wed, May 08, 2024 at 10:52:44AM -0700, Linus Torvalds wrote: > > > Example: > > > > > > static inline u32 __hash_32_generic(u32 val) > > > { > > > return val * GOLDEN_RATIO_32; > > > } > > > > But what about: > > > > static inline u32 __item_offset(u32 val) > > { > > return val * ITEM_SIZE_PER_UNIT; > > } > > What about it? I'm saying that your tool NEEDS TO BE SMART ENOUGH to > see the "what about". This is verging on omniscience, though. Yes, some amount of additional reasoning can be applied under certain conditions (though even that would still require some kind of annotation), but GCC can't even reliably figure out if a variable is unused in a single function. We're not going to be able to track overflow tainting throughout the code base in a useful fashion. > IOW, exactly the same as "a+b < a". Yes, "a+b" might wrap around, but > if the use is to then compare it with one of the addends, then clearly > such a wrap-around is fine. Yes, but it is an absolutely trivial example: it's a single expression with only 2 variables. Proving that the value coming in from some protocol that is passed through and manipulated by countless layers of the kernel before it gets used inappropriately is not a reasonably solvable problem. But we can get at the root cause: the language (and our use of it) needs to change to avoid the confused calculation in the first place. > A tool that doesn't look at how the result is used, and just blindly > says "wrap-around error" is a tool that I think is actively > detrimental. I do hear what you're saying here. I think you're over-estimating the compiler's abilities. And as I mentioned in the RFC, finding known bad cases to protect is the reverse of what we want for coverage: we want to _not_ cover the things we know to be _safe_. So we want to find (via compiler or annotation) the cases where overflow is _expected_. > We already expect a lot of kernel developers. We should not add on to > that burden because of your pet project. I think it's a misunderstanding to consider this a pet project. C's lack of overflow intent has spawned entire language ecosystems in reaction. There are entire classes of flaws that exist specifically because of this kind of confusion in C. It is the topic of countless PhD research efforts. People who care about catching overflows can slowly add the type annotations over the next many years, and we'll reach reasonable coverage soon enough. It doesn't seem onerous: I'm not asking that people even do it themselves (though I'd love the help), I'm just trying to find acceptance for annotations about where the sanitizer can ignore overflow. > Put another way: I'm putting the onus on YOU to make sure your pet > project is the "Yogi Bear" of pet projects - smarter than the average > bear. Sure, but if the bar is omniscience, that's not a path forward. I haven't really heard a viable counterproposal here. Let me try something more concrete: how would you propose we systemically solve flaws like the one fixed below? And note that the first fix was incomplete and it took 8 years before it got fully fixed: a723968c0ed3 ("perf: Fix u16 overflows") (v4.3) 382c27f4ed28 ("perf: Fix perf_event_validate_size()") (v6.7) Because the kernel didn't catch when the u16 overflowed, it allowed for kernel memory content exposure to userspace. And that's just the place that it got noticed. With the struct holding the u16 is referenced in 360 different source files, it's not always a trivial analysis. But, for example, the sanitizer not finding a "wraps" annotation on the perf_event::read_size variable would have caught it immediately on overflow. And I mean this _kind_ of bug; not this bug in particular. I'm just using it as a hopefully reasonable example of the complexity level we need to solve under. -Kees -- Kees Cook