From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f45.google.com (mail-wm1-f45.google.com [209.85.128.45]) (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 65E4F3E5581 for ; Tue, 31 Mar 2026 10:14:32 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.45 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774952073; cv=none; b=dso60xBRQPpCRGfVP0zoOyJ1QfHlNpLCpQQXOWkYghriXdQcdIQ2fkxSA+v5rW4AE3pGN2sXhlecGhyC6EcaC7hZUHLReYekeBuMfifMDBwfDPTj3JcGfHclVcUaIUx7v82mm79cfZJ18SC+vTvVA18FEIxYQpY81JyZMYOiXPA= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774952073; c=relaxed/simple; bh=pBJIPnybKnLg/DdwBPJji7V9mFWY9KFpzKug0waH6Ac=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=F/9XCFdkpRyAVWyLVumjxwl+8tO5rChN+QBGXGbX1/f451EbW7Iw4FgZNsx8SuxWrhhGSmwgIf4JDrye6h5+Q1Dx0eJy4MquWAblXdQXxL2vMG333xaqkZsefB6OGAt058xDmyHCH4wqpI7vG/xlkWRoGJvBFBXlxjmoq3GrZyI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=jZ2YSgSe; arc=none smtp.client-ip=209.85.128.45 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="jZ2YSgSe" Received: by mail-wm1-f45.google.com with SMTP id 5b1f17b1804b1-486fe2024a9so40185225e9.0 for ; Tue, 31 Mar 2026 03:14:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1774952071; x=1775556871; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=rHAguR9gpH9bNNCcSAAdxkGuK61fpqt8X9EW7Veme/0=; b=jZ2YSgSeLYlRefI7GcScWp5fKAnDWwiNg1GnxFb5Hgn80dosCDAvxAnRz7oUO3NSFk CwpQy85qSicoLJHzyK11bFVy0a4Z0phC0T0Mnif2DLaIWpuqr1B8ZIMlIrmeFKrlHRjV 6kEDctfqEYI9hJ4YkvcanrAjWksXdQnsOB0Z3lXUsFNyM5jLSPZBwQibhVkxpjRLbkV6 HQVorcb1YAQQp9hn+sRikCXZPpeZXnabp2Tvyu3oaEv9TkJ6B3nryrxRfuwsqfMtUaCi oanaPAyPmPHz/Guzu4o4drDGUrZWfhF+OebBfnATM/T1/4kAqRRvtnkCyTQu0gTD5PNP 0RBA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1774952071; x=1775556871; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=rHAguR9gpH9bNNCcSAAdxkGuK61fpqt8X9EW7Veme/0=; b=UYsppbRoV+etF45bfmWfWhBoNVeCPJUkH5vzLyqS260bXvm5akLE2vaBpNtsLOpbKT SIUvGmQCZA5fFYvO/oO4Zes9unht1ARUFLJFgHzlzoMYim16SOl/CsMw7f0rW9x6NE7L 5Tr3eJfojBb39/ZhiUHBoJslJfE8gsoRcqw3cVqT3q/a02qlQQvmxx1QAW5tm3OWboxo 7HrtdUhDxLVGrxHEpvyuEi2mnPQB/xwfiNUwWGzwaKz79Wu1Qyls+GiF4m4rpY4TwMSY pEaLXTqweDBxjOXVo0hwrG3RGr9xcbucq7pj6ecYthBSP3mcRXagMaptiv3ok/bdICHf EWjg== X-Forwarded-Encrypted: i=1; AJvYcCXc9Zin4fOqsvglCHsHEkQKUk1/eoXz42T3F9OKoMjWmpHzyQL6St4vq99uDY2qCsQa8LdoD2Lc5xv2z4I=@vger.kernel.org X-Gm-Message-State: AOJu0Ywj7ZtGmpKZ+bNFbxFYX6DOl4lrHGNwLQ6mf3PIxJDOsS6KiAHc 5PJTVxmCHWgdsukxEiWFxRawSiDoGUWQyuOgWpLhwkzyYkqOjMfTMJR/ X-Gm-Gg: ATEYQzz4udik3ppX7FVD4oM7dTmdoCR6v/iwbkfw8avoTnuZ8YBnUCIWNMgDFkVdrM3 5Ly90QOG0RfBxyx6/3ByymWYqkU2OCCVps85+YalBxK8VHJVvJRvZNPuKYi83xQW8PWFfH6bajd ZHi4mvzJ1Dfehge0QlfP5kbYm52R+2J2+GnY0LIEIHTcF2iDmEP1gzxUsqQhxEEVo8AjKhb7ErL WkaEabsYaD0KFRBdy9THpf9tiq2w5Rdijg/l8B0vk+b33oUEIZGOinWI+SoPrTp1GeorwAuU3dm RMhQtzbWjL0AZ4VGh4q5w6Ex7jGdOhalKL7fimiQQtsq5t03K6ri2/Czm0MYto8AJDEwGg38I9e D0103iDmFLGy087DpBTzw8LWQyEPwIXN8ntyzXKvvA6QOvr1j59NvpjTJ7tpg7wQrJBlEwDSewZ ZKXi/sBwyGkMJzVZ5Ay7aZIyV+Si03ku2p95V6gsbeYGdO8z8AG2qUmOkpJNZMiW1hroDDcKI= X-Received: by 2002:a05:600c:4e15:b0:477:7ae0:cd6e with SMTP id 5b1f17b1804b1-48727d5a16fmr270937105e9.5.1774952070406; Tue, 31 Mar 2026 03:14:30 -0700 (PDT) Received: from pumpkin (82-69-66-36.dsl.in-addr.zen.co.uk. [82.69.66.36]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4887e83682fsm26530635e9.7.2026.03.31.03.14.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 31 Mar 2026 03:14:30 -0700 (PDT) Date: Tue, 31 Mar 2026 11:14:28 +0100 From: David Laight To: Kees Cook Cc: linux-hardening@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH next 2/3] fortify: Optimise strnlen() Message-ID: <20260331111428.0b0575dd@pumpkin> In-Reply-To: <202603302335.0AEEF9154@keescook> References: <20260330132003.3379-1-david.laight.linux@gmail.com> <20260330132003.3379-3-david.laight.linux@gmail.com> <202603302335.0AEEF9154@keescook> X-Mailer: Claws Mail 4.1.1 (GTK 3.24.38; arm-unknown-linux-gnueabihf) 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-Transfer-Encoding: 7bit On Mon, 30 Mar 2026 23:36:07 -0700 Kees Cook wrote: > On Mon, Mar 30, 2026 at 02:20:02PM +0100, david.laight.linux@gmail.com wrote: > > From: David Laight > > > > If the string is constant there is no need to call __real_strlen() > > even when maxlen is a variable - just return the smaller value. > > > > If the size of the string variable is unknown fortify_panic() can't be > > called, change the condition so that the compiler can optimise it away. > > > > Change __compiletime_strlen(p) to return a 'non-constant' value > > for non-constant strings (the same as __builtin_strlen()). > > Simplify since it is only necessary to check that the size is constant > > and that the last character is '\0'. > > Explain why it is different from __builtin_strlen(). > > Update the kunit tests to match. > > See also > commit d07c0acb4f41 ("fortify: Fix __compiletime_strlen() under UBSAN_BOUNDS_LOCAL") > > -Kees > It is far more subtle that that; There shouldn't be a run-time access to __p[__p_len] at all. And you really don't want one. The problematic code was: if (__builtin_constant(__p[__p_len]) && __p[__p_len] == 0) If the compiler thinks __p[__p_len] is constant then it will also think that __p[0] is constant. So the extra check should really make no difference. I suspect this is what happened, consider: const char *foo; if (cond) foo = "foo"; else foo = "fubar"; return __compiletime_strlen(foo); This is first converted to (ignoring any silly typos): const char *foo; if (cond) foo = "foo"; else foo = "fubar"; len = __builtin_object_size(foo,1) - 1; // 6 - 1 if (__builtin_constant(foo[len]) && foo[len] == 0) return __builtin_strlen(foo); return SIZE_MAX; Since foo isn't constant that returns SIZE_MAX. The code is then moved into the conditional giving: if (cond) { foo = "foo"; if (__builtin_constant(foo[5]) && foo[5] == 0) return __builtin_strlen(foo); return SIZE_MAX; } else { foo = "fubar"; if (__builtin_constant(foo[5]) && foo[5] == 0) return __builtin_strlen(foo); return SIZE_MAX; } Since since foo is now 'pointer to constant' foo[] is constant, giving: if (cond) { foo = "foo"; if (foo[5] == 0) return __builtin_strlen(foo); return SIZE_MAX; } else { foo = "fubar"; if (foo[5] == 0) return __builtin_strlen(foo); return SIZE_MAX; } In the bottom bit foo[5] is well defined and known to be zero. In the top bit foo[5] is UB and gcc leaves the code it, giving: if (cond) { foo = "foo"; if (foo[5] == 0) return __builtin_strlen(foo); return SIZE_MAX; } else { foo = "fubar"; return __builtin_strlen(foo); } and you get a real reference off the end of foo[] - which UBSAN_LOCAL_BOUNDS rightly picks up on. clang has a habit of silently deleting everything after UB, so might generate: if (cond) { return whatever_happens_to_be_in_ax; } else { foo = "fubar"; return __builtin_strlen(foo); } The 'fix' of checking __p[0] actually makes no real difference. I'd guess that the longer code block stops gcc moving the code into the conditional and hides the bug. But that could easily change by just breathing on the code somewhere or in a future compiler version. I suspect this should be a compiler bug. But with the compiler behaving this way you can't write __compiletime_strlen() with a check for the '\0' terminator. That really means you can only use __builtin_strlen(). Which means you'll get a compile-time error from: char foo[3] = "foo"; __builtin_strlen(foo); rather the 'not a constant' when checking strscpy(tgt, foo, 3); At a guess that never happens except in the tests. David