From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from fhigh-a7-smtp.messagingengine.com (fhigh-a7-smtp.messagingengine.com [103.168.172.158]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id CB9CB199E94 for ; Fri, 7 Mar 2025 08:58:43 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.158 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741337926; cv=none; b=Xcv1xQCVa/PEXI8QBiqsihz0Zk+b9oTIoFDjmpS6IFsH7q8FVQwNW5ZNxrp0m4NbSlm7cFclwGNRskd26MUPRYPTwjqTSYcg56cFPaklo3+VD19FaSfXaL+oFJixuduodv3UQepCfht9NfQH6bfb833Q7uqhpltWb+KurP7Wplk= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741337926; c=relaxed/simple; bh=ZjIQfWG83oRMyHUQO4OLqSgSMHEWjUJSojUPoWkoC0c=; h=Date:From:To:cc:Subject:In-Reply-To:Message-ID:References: MIME-Version:Content-Type; b=dh4QIZDaDonKJANvDXPH1NrhOY/DqTXu8fJ/lyCWVT+5fK2CDU23M+Jmdy6gsu83e22pPAxFq+IXsDDWGvmFpMkXRB1303+97Y+MAAk38ScRL+GQRSX2nw7zU9vALFzun9RRYzgz/8KZ8yptfLXqRZw+EktJzGITqVX5JpitHFI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=linux-m68k.org; spf=none smtp.mailfrom=linux-m68k.org; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=tQ7yE1Eg; arc=none smtp.client-ip=103.168.172.158 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=linux-m68k.org Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=linux-m68k.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="tQ7yE1Eg" Received: from phl-compute-09.internal (phl-compute-09.phl.internal [10.202.2.49]) by mailfhigh.phl.internal (Postfix) with ESMTP id C62CB11401BE; Fri, 7 Mar 2025 03:58:42 -0500 (EST) Received: from phl-mailfrontend-02 ([10.202.2.163]) by phl-compute-09.internal (MEProxy); Fri, 07 Mar 2025 03:58:42 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t= 1741337922; x=1741424322; bh=QPeVGTBd43rKn7LkcJDA/FXYNYp2iIcX4WE UvEUfYNM=; b=tQ7yE1EgJ60/UNnooVX4mQmvkVu0xuEswZPUEIJNovrTsWDaOeV +pxZQlK85pyoGpROjrehmBJPfQN80xCqPyan4mGrhB/0ELwTVei5pVUhIQGnN8Z/ n8ZA6Bl72ujJt7nM+GAAcPWGaPFst0Uoc0fjskdktQyEnkyDydJklOF8vljQI5DH HTCqstREL9Nd3jLw7iNhFzsCJH8a5mzO8a2N3VMJLrijeFc5sBnDleWM7cpZ5kTE A0rsTUZhZnLfqBIV673rUZF2ErQsf8o4Vk8eiFrFS2OQUTgo8WJys66QRlX8VYEb YGzHlpTPymXoMMR4dTILW+UV8gFPsbVuTlA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgdduuddtvdehucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggv pdfurfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpih gvnhhtshculddquddttddmnecujfgurhepfffhvfevufgjkfhfgggtsehttdertddttddv necuhfhrohhmpefhihhnnhcuvfhhrghinhcuoehfthhhrghinheslhhinhhugidqmheike hkrdhorhhgqeenucggtffrrghtthgvrhhnpeelueehleehkefgueevtdevteejkefhffek feffffdtgfejveekgeefvdeuheeuleenucevlhhushhtvghrufhiiigvpedtnecurfgrrh grmhepmhgrihhlfhhrohhmpehfthhhrghinheslhhinhhugidqmheikehkrdhorhhgpdhn sggprhgtphhtthhopeeipdhmohguvgepshhmthhpohhuthdprhgtphhtthhopehgvggvrh htsehlihhnuhigqdhmieekkhdrohhrghdprhgtphhtthhopehthhhorhhsthgvnhdrsghl uhhmsehlihhnuhigrdguvghvpdhrtghpthhtohepjhgvrghnmhhitghhvghlrdhhrghuth gsohhisheshihoshgvlhhirdhorhhgpdhrtghpthhtoheplhhinhhugidqmheikehksehl ihhsthhsrdhlihhnuhigqdhmieekkhdrohhrghdprhgtphhtthhopehlihhnuhigqdhkvg hrnhgvlhesvhhgvghrrdhkvghrnhgvlhdrohhrghdprhgtphhtthhopehkvggvsheskhgv rhhnvghlrdhorhhg X-ME-Proxy: Feedback-ID: i58a146ae:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 7 Mar 2025 03:58:40 -0500 (EST) Date: Fri, 7 Mar 2025 19:58:39 +1100 (AEDT) From: Finn Thain To: Geert Uytterhoeven cc: Thorsten Blum , Jean-Michel Hautbois , linux-m68k@lists.linux-m68k.org, linux-kernel@vger.kernel.org, Kees Cook Subject: Re: [PATCH] m68k: mm: Remove size argument when calling strscpy() In-Reply-To: Message-ID: References: <20250302230532.245884-2-thorsten.blum@linux.dev> Precedence: bulk X-Mailing-List: linux-m68k@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII On Fri, 7 Mar 2025, Geert Uytterhoeven wrote: > On Fri, 7 Mar 2025 at 00:24, Finn Thain wrote: > > On Thu, 6 Mar 2025, Geert Uytterhoeven wrote: > > > On Mon, 3 Mar 2025 at 00:07, Thorsten Blum wrote: > > > > The size parameter of strscpy() is optional and specifying the > > > > size of the destination buffer is unnecessary. Remove it to > > > > simplify the code. > > > > > > > > Signed-off-by: Thorsten Blum > > > > > > Reviewed-by: Geert Uytterhoeven i.e. will > > > queue in the m68k tree for v6.15. > > > > The commit message says "simplify the code" which is only true if you > > never scratch the surface (i.e. it's simple code if the reader is > > simple too...) > > The code is simpler in the sense that the API is simpler to use, and > harder to abuse (i.e. to get it wrong). > > > Commit 30035e45753b ("string: provide strscpy()") was a good idea. It > > was easily auditable. But that's not what we have now. > > > > Patches like this one (which appear across the whole tree) need > > reviewers (lots of them) that know what kind of a bounds check you end > > up with when you ask an arbitary compiler to evaluate this: > > > > sizeof(dst) + __must_be_array(dst) + __must_be_cstr(dst) + > > __must_be_cstr(src) > > > > Frankly, I can't be sure. But it's a serious question, and not what > > I'd call a "simple" one. > > All the __must_be_*() macros evaluate to zero when true, and cause a > build failure when false. > It seems to me that the code review problem could be solved either by not churning the whole tree, or if we must have the churn, by short-circuiting the recursive search by reviewers for macro definitions. Can we do something like this? sizeof(dst) * !!__must_be_array(dst) * !!__must_be_cstr(dst) * !!__must_be_cstr(src) At first glance multiplication appears to be safe (unlike all the addition terms that we have) because the limit of the string copy is either unchanged or zeroed. Yes, I know you said "zero when true". That looks like another design flaw to me. But maybe I'm missing something that's more important than readability and ease of review. > BTW, Linux does not support being built by an "arbitrary compiler": only > gcc and clang are supported. > So only gcc and clang must agree about all of the details...