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 A937834F46B for ; Wed, 18 Mar 2026 16:33:39 +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=1773851619; cv=none; b=X4jU26MsD+BJ1I4sn6mLiXub/s1T4GHYfHFYi7OUZxVQ6dub+oUhOumXPCR5vf7RkHSSjwP1oSXzuHg9e7p5qe+qUuQiePaLi+yxfKCR51ihKVVGpiwdpCcsCGs6Uc0JCcrcihYc/BzcTWA4/FvCg5Q8nfYL4IAh+z4kf0f5qdE= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773851619; c=relaxed/simple; bh=ntHD+xKjsEzzHIS4Xtvpb62MBVywvFGS4+7I5zMj4Lc=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=l5jUBt6UV0a9cSNnnRTRk8McaiAI1dZoU4EKzPgSLZQnuvzeNO5GBoBUwK5i/LSp0e6TnzD/nJrXyq05Sqi/6t4ZsqUnCi7h8olldE0qqR5jxiHrjKoIDPemltU9IRxx7sj48VVsDLzGSTJL5YCOT11abNAkst92byjO6/CyFfY= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=dUSJS5W3; 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="dUSJS5W3" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 038E7C19421; Wed, 18 Mar 2026 16:33:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1773851619; bh=ntHD+xKjsEzzHIS4Xtvpb62MBVywvFGS4+7I5zMj4Lc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=dUSJS5W3e/T0Ql/at321NF1h1VpWSK1QBP4LBYPGFvx1EGTrtUWrR3IDB240SLHac +8FHGuQpY8QmFGLRkluY3bc/GC/MMtKDSUJ/loKiML+WPekRjatDTEG1BD1vvaLIUp 1WBCeHuy+HH50K2iSURw4HUb60d9HOXn/Ul5QXVVasL9wU0d6LsqCNbmAY5/iNpNL8 SQJqzAeu1TrRunseDelGejVi0o+wxgfiQEU7h3zTHv/ilWCpuKLNe3rW359YIm1t0n IWx1kzopuXUkPiBsKtG9mVbF9oAoxb8t+YbI48DDpDmge5mqNpfy9mxPprQ53ss907 NO9WWGXhyRgaQ== Date: Wed, 18 Mar 2026 17:33:35 +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="mksc7zprxfqxp7wv" Content-Disposition: inline In-Reply-To: --mksc7zprxfqxp7wv 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: Hi Kees, On 2026-03-17T22:40:53+0100, Alejandro Colomar wrote: > On 2026-03-17T14:14:11-0700, Kees Cook wrote: > > On Mon, Mar 16, 2026 at 07:33:34PM +0100, Alejandro Colomar wrote: > > > 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 t= ype > > > ptr =3D malloc_T(n, typeof(*ptr)); // But we'd really pass the t= ype > > >=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 see= ms > > > 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. >=20 > 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. >=20 > I understand the empty comma might be more controversial; although > I hope we can eventually discuss it too. >=20 > > > I think the combination of having the macros be variadic (for gfp) wi= th > > > 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. >=20 > Indeed; I agree Linus's change was the worst part. >=20 > 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. >=20 > > > 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. Tha= t'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". How about using a struct? That's the idiomatic way of having incompatible types. Since these are used through macros, it wouldn't change anything to users. The macro definitions would have to change. For example: -#define GFP_NOFS (__GFP_RECLAIM | __GFP_IO) +#define GFP_NOFS ((void)0, (struct gfp){__GFP_RECLAIM | __GFP_IO}) We don't need any new language features. This is backwards compatible up to C99. And it might end up being simpler than __strict typedef. > 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. >=20 > 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. >=20 > > > 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. >=20 > 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. After sleeping, I had some idea. We could have coccinelle add typeof() around the first parameter when it's an expression (not a type). Then, we could enforce that the first parameter is a type name. That is: p =3D kmalloc_objs(int, 42); // ok -q =3D kmalloc_objs(*p, 7); +q =3D kmalloc_objs(typeof(*p), 7); I expect this would be doable with coccinelle. Then, new code would be required to pass a type name. And people could slowly replace the existing typeof() calls at their own pace. What do you think? Have a lovely day! Alex > > > require a type name, and a variable is rejected. We implement that w= ith > > > 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. >=20 > Nice. >=20 > > > But there's also a safety aspect. Consider we want to allocate an ar= ray > > > 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()". >=20 > 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. >=20 > Maybe you could diagnose with coccinelle where expressions are used? > Then people could slowly look at their diagnostics and replace them. >=20 >=20 > Have a lovely night! > Alex >=20 > --=20 > --=20 --mksc7zprxfqxp7wv Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEES7Jt9u9GbmlWADAi64mZXMKQwqkFAmm609gACgkQ64mZXMKQ wqmcHg/9E4EcNu+X6qVC/sdMH8mB15+gfr18uaP3T8Hi4mOfVPkarYXdsBoXwe7c srnrlIfAIqP4itpTgs1UGGG824uGbB7UZmxoWk8muzMs3GRywSp/HtNNlbqYX3Xs jP8tfSXY5XqE+uvehiDclNGtT1TF0dY5FB+dvgR4swlDgT4bdCj/7GoOXYIltjd/ nOVKoZCJm8J8JWIvlU2/rSa3MDwr0gKTuk+WnMFJMb6J+7YBU0Ugx/WYanCgFlOf m4CBzn8QjFWIvwIFaPxvJG3LRrX5K6mJLRBww1p0eb3XFClN1/zpPUwyDJ+pmvW4 k7GQnTPkS99DU/HibZkQY3DYQ/ot8TP8Aa8Av+XaoXTlXScbn4+EvqESAbnZ/Bg8 9KsK/jpH6ZnD+0s/yw1imuGRY7ZZRmrLx8DCut9k3YE8jxwxfEcwukLHJZrxvBvp DDidwTTC7c5O2hn3ZZcu+p9fIzr00CdVq7JEtNuTve42+AbpQ0D0on94g9D7TChm 5rMwhmvn84fexnAHOblJAJZPSMAtwF+IrCr96YEJA1bk7tgGT6lmxmix5McP66Kg oyTLFJg4kFiDeIQDOKEttjJwYZHux7mcd+bME9GJV5SK1B9JpOJRTAtknTsCBuH0 8gJ0+shkhgOKCZLplBIzwFHXg4/n80RgxlU2c2TTg0HX3VdYRWo= =K/hR -----END PGP SIGNATURE----- --mksc7zprxfqxp7wv--