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 A5D1339150E for ; Tue, 17 Mar 2026 21:40:53 +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=1773783653; cv=none; b=sHC3zYaCKD5He8RJuyLcAL3oPyspzXAzM2Qs8EiUM6EugQ7nHkenXmfq2NxQ8sxyfi3VPjngAWbrBxrp5IpfNU1Jbz4VJ/mzP/+c1tkAS/da07VrOQcXX64ISvrSuEZVDSyhcc3UiB4eV+Aitq6d9RnBQkbECpYlvGSZnE/8+Qk= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773783653; c=relaxed/simple; bh=EuWGrE5qv+fBKRGiChebrDZbh0JB1/PPi0HCa2S4xT4=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=fvsm/9s1WaVFXWtehGKP9rO85zh/axVmEwWIwnCT2/dR4IohS5+O3vkpDtH8lKaUFwaKlfwsT5NiUTx8XQncvt4kRM399pJXZCbfIgOHiffufp9ISiBP6cCDeTDZW54U5XFeKT2McHf+jhzuEumYkdxrdMQFwqUNaJ059W15nLc= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=LfJg7vd3; 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="LfJg7vd3" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 3A63BC4CEF7; Tue, 17 Mar 2026 21:40:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1773783653; bh=EuWGrE5qv+fBKRGiChebrDZbh0JB1/PPi0HCa2S4xT4=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=LfJg7vd3icn2vNrm73w5OK3IAPHHYuh1EeyMn+8vthsa/cpwivL8uc14MLwlXPsFH ydJWgW4YsxY2Eiocf5FvnILkVRva+/WxUpu8i7gvHNDCSTdb3OQ3nEQXsrss9jFfhD cpT0H+ML0zKjdX/SNMtgp58fNL3wVHHA1W9Uk+R9lqrpb+XvyGtgxNAP3GEvvyyMwH 9trkJ4Dz5C67nT5pPppyw0OsaLjIOCoSgrP6qcTrKIUis+vakkQYkBopaQ00JplUu0 u/f/veIj8t+XX6k7jaozcKmGnR0LKYVAng1XzzycaetoNR2iMRI8z+d7KIzmlSYGtL Rz6xj0JESmAeg== Date: Tue, 17 Mar 2026 22:40:49 +0100 From: Alejandro Colomar To: Kees Cook Cc: LKML , corbet@lwn.net, serge@hallyn.com, Martin Uecker Subject: Re: kalloc_objs() may not be as safe as it seems Message-ID: References: <202603171402.B2BD1B1@keescook> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="qxlafnjy5zlkxcpu" Content-Disposition: inline In-Reply-To: <202603171402.B2BD1B1@keescook> --qxlafnjy5zlkxcpu Content-Type: text/plain; protected-headers=v1; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable From: Alejandro Colomar To: Kees Cook Cc: LKML , corbet@lwn.net, serge@hallyn.com, Martin Uecker Subject: Re: kalloc_objs() may not be as safe as it seems Message-ID: References: <202603171402.B2BD1B1@keescook> MIME-Version: 1.0 In-Reply-To: <202603171402.B2BD1B1@keescook> Hi Kees, On 2026-03-17T14:14:11-0700, Kees Cook wrote: > On Mon, Mar 16, 2026 at 07:33:34PM +0100, Alejandro Colomar wrote: > > Hi Kees, > >=20 > > I just learnt about kalloc_objs() et al. from > > . > >=20 > > ptr =3D kmalloc_obj(*ptr); > > ptr =3D kmalloc_objs(*ptr, n); > >=20 > > This resembles a lot the macro we have in shadow-utils, malloc_T(), > > which would be used as (to resemble the above): > >=20 > > ptr =3D malloc_T(1, typeof(*ptr)); // But we'd really pass the type > > ptr =3D malloc_T(n, typeof(*ptr)); // But we'd really pass the type > >=20 > > But I've noticed some design mistakes that make it not as safe as it > > seems. > >=20 > > Default arguments > > ~~~~~~~~~~~~~~~~~ > >=20 > > 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. >=20 > 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. Should we add Linus to CC? I think the safety arguments, especially after his patch adding the default argument, talk strongly about using the literal 1, and dropping the _obj() macros. I hope the "1" is something people could consider. I understand the empty comma might be more controversial; although I hope we can eventually discuss it too. > > 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 > >=20 > > ptr =3D kmalloc_obj(*ptr, n); // Bogus > > instead of > > ptr =3D kmalloc_objs(*ptr, n); >=20 > 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. Indeed; I agree Linus's change was the worst part. Having just one of these two issues separately would be relatively fine, and it's the combination of both that makes it such a foot-gun. From the two issues, the variadic argument list is probably the most dangerous part. > > 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: > >=20 > > ptr =3D kmalloc_obj(*ptr,); > > ptr =3D kmalloc_objs(*ptr, n,); > >=20 > > 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. > >=20 > > With mandatory empty arguments, the compiler would easily distinguish > > mistakes like the one above. >=20 > 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". Yup, that would help too. Although in general I still think that variadic argument lists are very dangerous: there's no difference between "the programmer forgot to specify the argument, and the compiler didn't remind them" and "the programmer wanted the default value". This is a source of long-term issues. I understand the extra comma might be difficult to sell at the moment, but we should try to figure out a way to sell it in the long term. > > Type safety > > ~~~~~~~~~~~ > >=20 > > 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 >=20 > 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. >=20 > Making the refactor "trivial" was important; but see below. Sounds reasonable. This imperfect API is still (or was, until Linus turned it into a double foot-gun) better than what was before, and if it serves as a step to eventually reach the better APIs, it's great. > > require a type name, and a variable is rejected. We implement that with > > the typeas() macro: > >=20 > > #define typeas(T) typeof((T){0}) > >=20 > > 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: > >=20 > > // malloc_T - malloc type-safe > > #define malloc_T_(n, T) \ > > ({ \ > > (typeas(T) *){reallocarray(n, sizeof(T))}; \ > > }) > >=20 > > which is used as (taking some arbitrary examples from shadow-utils): > >=20 > > lp =3D xmalloc_T(1, struct link_name); > > targs =3D xmalloc_T(n_args + 3, char *); > >=20 > > Some reasons for passing a type name instead of a variable are: > >=20 > > - 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. >=20 > 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. Nice. > > But there's also a safety aspect. Consider we want to allocate an array > > of 42 ints. And consider the programmer accidentally swaps arguments. > >=20 > > int *p =3D malloc_T(int, 42); // syntax error > > int *p =3D malloc_T(42, int); > > vs > > int *p =3D kmalloc_objs(*p, 42); > > int *p =3D kmalloc_objs(42, *p); // Bogus > >=20 > > 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. >=20 > 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()". Agree. The good part is that the refactor doesn't need to be as careful, because incorrect types will result in build errors, and not bugs. But yes, it needs significantly more work than what the current API required. Maybe you could diagnose with coccinelle where expressions are used? Then people could slowly look at their diagnostics and replace them. Have a lovely night! Alex --=20 --qxlafnjy5zlkxcpu Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEES7Jt9u9GbmlWADAi64mZXMKQwqkFAmm5ylwACgkQ64mZXMKQ wql3+hAAiebi+ogMggentE4qzMnhyxxa5KSMwKB8tm9ccKavfQbrcz5u7k6I0FFo YVrCxNIqqsYWKxYRi5HJ1WA726ik5ptJM+L9dhSGqC3p9Nj8Zwl6FpfrD2+8WcsN DJ2jTqVBVWSS7BzB4NxESWKyocnTIAVEoCYxhsU7oQ1qRmipzLvc/mB387vzctHU 16TCWYXcsttTS95vhkQqmSBFTkx9vF+11ZzbDqhPUG1YM10HZrrd9Kot00y8rWFb h+o2plkEuMkVIOM545Z1hT6wrNCSAqsBqoH7U9SU/+fnysNETobY+BFJub4zfbjE TBgBto4ClTjc5h7pD943FhWnm++Vk8nmDfE6nwhNAsTvOE2rhBt6vxzAa7bqQDHK 6fFQN9udgbw9JxPr2CzBIpeqosadQUUxtc23/Z+82MgqJbY2t2fGMHu+xne4Nlr+ tePcp/s+WSaVgm43ig627+gSrv3/hA//6TeHhhL74rsjZvyX8+sIsYKCXibX2QM2 iPqxe62rHJ60/fbdfa7pMb2fUOH7Wwrb1Sx5wrQ4dzrjlwZSbZxyjEhTabrampeO MiIr91CttiXrs34gSKxOEw1Mmf2Xrvd7fYJjFXmw7qLH8HXnTSRUR3Ue851LhMTs lqLxWZ5VEpJTDzliFtt3K/K0MufXFum2TKtGAwv4fEyC2o0dstQ= =njbL -----END PGP SIGNATURE----- --qxlafnjy5zlkxcpu--