From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (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 2A2D538F92F for ; Tue, 17 Mar 2026 21:14:11 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773782052; cv=none; b=SjTqxbE4n2yKc2IaOjf6+TQTm/JjlypZrbc44dFPyWlVGTveaj9VJlZcMpUA0V6PTXSOO2YkNCFvrk+mJtlPsHpXOznaXOpk/4N3FE/XF9dcLopE5d9QbBWEIlqRZISFC0l+0DD4/lV7pzmD9vTz1WCixNOzeE6oQs7wPsbBSJc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773782052; c=relaxed/simple; bh=jl9WUkp/X0dSncBg1GhamNtkQH4xjxdadoTPxasatBs=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=CF7n3OpqVd7ZW3wgw0Kqc4zRi+KOUvphvOsqGod//oZ01hOvIRU1dUEJ5F6KeTjKYALw01twDCYHU8TGsgAqUdVh5+ZWRxkSHwyBczRVcitH1o84OznER5C51J93uSBoaMMJwOevL+Thk9a8jShgpPk1kp0zvLi+pE5M4VLLMrs= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=UvH2S+LK; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="UvH2S+LK" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 873CDC4CEF7; Tue, 17 Mar 2026 21:14:11 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1773782051; bh=jl9WUkp/X0dSncBg1GhamNtkQH4xjxdadoTPxasatBs=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=UvH2S+LK6xpu40hk5PY+LLtIgz4J/sSrJUuR4tZ8UdjMmfJdzTvlKSuBlnP+m3A4L Q7BoPbHYXVd5sUzROunvOp6uPUSiLwAUQ2S9vFcbGX32wqMArS9ugaFHAB2VOOIgZf iNz3atV3npxozZabOLW8a1NO2NR3I8qqtHvVQvKWf0sdknNNdQCbqpb7mavT+WPuTr hFC1y1QbxtDq7ozovRGcU2FxtOZ3v2zvoSaqVlOMnEN4B+ksEf3euazc3vDtFHNjP+ mxb9cQq0mbf+PrQlW8xNqh3i6Gn9ec66AkZq2dyFb2oyk1avkVYvqQHFxaZWYOO9od WSaJcFp7v5N/A== Date: Tue, 17 Mar 2026 14:14:11 -0700 From: Kees Cook To: Alejandro Colomar Cc: LKML , corbet@lwn.net, serge@hallyn.com, Martin Uecker Subject: Re: kalloc_objs() may not be as safe as it seems Message-ID: <202603171402.B2BD1B1@keescook> References: 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: On Mon, Mar 16, 2026 at 07:33:34PM +0100, Alejandro Colomar wrote: > Hi Kees, > > I just learnt about kalloc_objs() et al. from > . > > ptr = kmalloc_obj(*ptr); > ptr = kmalloc_objs(*ptr, n); > > This resembles a lot the macro we have in shadow-utils, malloc_T(), > which would be used as (to resemble the above): > > ptr = malloc_T(1, typeof(*ptr)); // But we'd really pass the type > ptr = malloc_T(n, typeof(*ptr)); // But we'd really pass the type > > But I've noticed some design mistakes that make it not as safe as it > seems. > > Default arguments > ~~~~~~~~~~~~~~~~~ > > I tend to think it's simpler to have a single API that works for both > 1 element and multiple elements. The special-casing of 1 element seems > unnecessary, and having a literal 1 works just fine. This is a reasonable opinion, but not one I wanted to try to fight for with the Linux developer community. Linus has tended to frown on adding any new burden when making these kinds of changes, and expecting everyone to add a (seemingly) redundant "1" to all API calls (or an empty comma) seems unlikely to fly. > I think the combination of having the macros be variadic (for gfp) with > having two very similar APIs that differ in number of arguments, and > all those arguments being integer types, is prone to errors. Consider > the case where one would accidentally write > > ptr = kmalloc_obj(*ptr, n); // Bogus > instead of > ptr = kmalloc_objs(*ptr, n); This loss of GFP flags wasn't part of my original design, and I would agree that given the lack of type checking for GFP flags, this does look like a potential foot-gun. > The compiler wouldn't realize at all. That's a strong argument in > favour of having default arguments be required to be explicit, with an > empty argument: > > ptr = kmalloc_obj(*ptr,); > ptr = kmalloc_objs(*ptr, n,); > > I know you (and Linus too, FWIW) have previously claimed that it looks > weird to the eye. But I'm pretty sure you could get used to it. That's > certainly going to be safer. > > With mandatory empty arguments, the compiler would easily distinguish > mistakes like the one above. I'd rather we get something like the __strict typedef so "gfp_t" would be a true separate type, not just a silent alias of "int". > Type safety > ~~~~~~~~~~~ > > Apart from the issues with the above, the ability to pass a variable > instead of a type name is also a bad choice. In shadow-utils, we This was a first-order requirement, or we'd never be able to refactor the codebase to use the new API. There was already a heavy mixture of types and variables used within sizeof(), and trying to take that part and find types exceeded Coccinelle's abilities. Making the refactor "trivial" was important; but see below. > require a type name, and a variable is rejected. We implement that with > the typeas() macro: > > #define typeas(T) typeof((T){0}) > > This macro works exactly like typeof(), but it requires that the input > is also a type. Passing a variable is a syntax error. We implement > malloc_T() with it: > > // malloc_T - malloc type-safe > #define malloc_T_(n, T) \ > ({ \ > (typeas(T) *){reallocarray(n, sizeof(T))}; \ > }) > > which is used as (taking some arbitrary examples from shadow-utils): > > lp = xmalloc_T(1, struct link_name); > targs = xmalloc_T(n_args + 3, char *); > > Some reasons for passing a type name instead of a variable are: > > - It allows grepping for all allocations of a given type. > - It adds readability. It's similar to declaring variables with some > explicit type, vs. using 'auto' (__auto_type) everywhere. Sure, and that's why many places were already using type names, and the use of "auto" was even one of Linus's driving examples for why the new API could be very easily used. > But there's also a safety aspect. Consider we want to allocate an array > of 42 ints. And consider the programmer accidentally swaps arguments. > > int *p = malloc_T(int, 42); // syntax error > int *p = malloc_T(42, int); > vs > int *p = kmalloc_objs(*p, 42); > int *p = kmalloc_objs(42, *p); // Bogus > > The latter is dereferencing an uninitialized pointer. If for some > reason the pointer had a value before this call, you'd be allocating as > many elements as *p says, which would be bogus, and since typeof(42) is > the same as typeof(*p), the return type would be valid, so this would > still compile. Yeah, this is a foot-gun too. I'm open to requiring a type, but it's a significant amount of careful refactoring needed to accomplish it. It was a weakness of the existing API, though it was more "obvious" since it was visually contained by "sizeof()". -Kees -- Kees Cook