Received-SPF: pass (google.com: domain of linux-kernel+bounces-34861-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <53a886c9-10df-4bc5-9de2-d9f99ea78278@prevas.dk>
Date: Tue, 23 Jan 2024 09:03:10 +0100
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 02/82] overflow: Introduce add_would_overflow()
To: Kees Cook <keescook@chromium.org>, linux-hardening@vger.kernel.org
Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>,
 Bill Wendling <morbo@google.com>, Justin Stitt <justinstitt@google.com>,
 linux-kernel@vger.kernel.org
References: <20240122235208.work.748-kees@kernel.org>
 <20240123002814.1396804-2-keescook@chromium.org>
Content-Language: en-US, da
From: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
In-Reply-To: <20240123002814.1396804-2-keescook@chromium.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?QXFnN3lrcGxyeWRuVjNnTUVOMDA5S1ZucnZseHFGVlRoV05zRTdBQlFSZUha?=
 =?utf-8?B?MTdaWWJoVDNQaHZucmxpU1M3TEVXb3UySzJ2VUpHQm9hYzNhU0hqM1NyV0c4?=
 =?utf-8?B?ZzJDSVFaOGY4azJJTTAreHhHcEswM3pURVFqYVY3d3JGUTFwM2NMRVkwNFNC?=
 =?utf-8?B?aGRWUHNybHYySTBPWmhKRTN6M2pTMzNrZ0sxRE9KU1pmaFdnYnBQM1FVSjFW?=
 =?utf-8?B?NG9KRUxhNjRjR3hLdTZRWmt1M1Q3KyttZGhIcHRxMTJRcUxGdFVha0RnR3ZV?=
 =?utf-8?B?anpXUmNpZnY1V2s0SVNZQjE3a3dzNjd0L0tRNURUblJtNFk0bHhISjg3cWl2?=
 =?utf-8?B?b3hjR3NUcmpZdFZNM3NydFU1a2tJVG0rcVlxdG5FdWc4MmFrc2JGSW9KL1p4?=
 =?utf-8?B?VHNwOC9TZ3RJYXlqaTY2SU5CaXhMZGxueENGdlMwbC9uM2JiYnFJTWgzemo4?=
 =?utf-8?B?aWk3TG1HNnNuWlVnR3VLQllzYWk5VXZaMzhmVEMzL2dBZGJrbFZ4MHB2bHRy?=
 =?utf-8?B?RDlFNU5mSStkTUo4SUxUQVUweFAwYjRjbEZiNXNCckFUWW5hdUdLZS9ydjF2?=
 =?utf-8?B?UjZRdkRYR3JVcGc2OFBaZTZLN3haRzkwSjd4Vko5RHhEczhKdGc1UjNUeGs2?=
 =?utf-8?B?K3FMeUp5RS9LZHhOUmNiaG16aGF6eUVhaG8zalNCcmxOalMrOGUrMjB0L3pQ?=
 =?utf-8?B?dXpNS1RUMUE5U1JsblJvdTZBM0lKS0RpZnhBZlV6ekZMMW9tNUdmYjBtaW5C?=
 =?utf-8?B?MEpEQXE2QmtxVmRXeGtBUHhkVEQ3R0tQRDladmFFd2VxOUVaTUZnamVqd21R?=
 =?utf-8?B?NGd0MERNRE14VGxESnZ2U2VpM2pQbGhFVW52LzBjZXY3MEVFcXExNFlzWUw3?=
 =?utf-8?B?dHBhcy9GNHhJdVVNKytEMTZnU3JWYjRaalRKS09DcEc5em9Cbk1SSDhZTVpL?=
 =?utf-8?B?UHk1MzcvT0Y0MGo1MFRLUHRISUhPa0FXRThzYUtnNUYvY21jY2JvVHlvRERG?=
 =?utf-8?B?eDFrZjhoVDNuaHcrQ2ZsUm5zRUlkTmpvYUVPY245MDVwLzBEVzV1K1daNTJv?=
 =?utf-8?B?eVFlVjBCcElVd0NkRXkrN1RSc2UvWGo3TE9FTWxBQWxtS01oYVR5bmdMT1VS?=
 =?utf-8?B?TVZSalhuNWdjU1hHbE5iU1dwSjVPbTdTRTRFN2tMdWxNemxnd1dPQ3JSU1pt?=
 =?utf-8?B?QkhrenZwVmE5UDJQUjhJU08zQXJlV3JsOGwvTXBzd2hlWjlHVTJmaGJpQTdV?=
 =?utf-8?B?ckx1U2tFUURUQysxWndCYldvOWlTQmVvczRLMm8wUVFFYW5JOGIzRXRSZG9Z?=
 =?utf-8?B?ZXBtd3lPUWI1dThLL1Flb2lHZGhLbWNZNy8yK2tNeHYvVjFaVTJDS0dxZDNI?=
 =?utf-8?B?dnlsL01tQ2hyVmorRDFCYTVHNGE4QlhhVTdxc2RKSUpoTE5rVUNveGU0Zk9E?=
 =?utf-8?B?eHc1TFZYb3NjQWRzUDJvZlkzcEFrUHp3R1RKUzJxL0RYdUY5NHZ5NzN5enBM?=
 =?utf-8?B?WkFUcnVaMHhWTmF2YzNmSnRtSENSdnFpcXRFL3pKd1hpR2ZLNERuZzBiWUV0?=
 =?utf-8?B?Z0tlNU9ZYkJRdkhQbFBGVUw2M0VrejJtSU1HbW5RMjlSVUJ6UVp3VFlhRmZ3?=
 =?utf-8?B?aitVV3BMNnN0WE9pTkNJUXdJT0Q3c3lRNjFNaUVwYVBmRENZbmpsQ3Jmcm1z?=
 =?utf-8?B?UTBCQmthUTUraVVrU0NJRmtvVmFjSVZsUzk3aFFHem5XWURWZ1kxWCszclpC?=
 =?utf-8?B?NVZyY0ltVlZ2aHA4TVdtNjQ0dHNKVU9DSk9zRmZUcEMrV0xaYk5PaXdkYS9k?=
 =?utf-8?B?cmdMZ2p0K3RKS0cwUnZmWkRzOUwvZDhhRjlramk1dHhBWEp4SG1JK1JpTjBr?=
 =?utf-8?B?TnhGNTN2UjBSaVd3TUlWeE4vMHJCQ1poblRmVStTenlUaHFLMno2VXhreXVx?=
 =?utf-8?B?WkZXdkpSOGhYSTVOTWVYWHlvS2RMZUJubEVRUWNGRkRqaHd0UWszVUN6Nzd2?=
 =?utf-8?B?OC8rd3JXWkpWQmxCS2pxd1VrVFJpb3R1amdCOXAxRTVjZmh3bTQ4WE5ZNWtx?=
 =?utf-8?B?cWwrNC9kUXYrZXU0Ky8yNE5aVzNpS2JFWWg2cm1jcXljclUvRld0N3ZySkx2?=
 =?utf-8?B?TlZ5STRRQWtOQTNwWjZxeHFwa1FsYXRRNDMyQ3pNSTFiYjZYQVQwRUtSRHRh?=
 =?utf-8?B?TXc9PQ==?=
X-OriginatorOrg: prevas.dk
X-MS-Exchange-CrossTenant-Network-Message-Id: 678c4a6f-42f8-4eaa-e904-08dc1be9bf50
X-MS-Exchange-CrossTenant-AuthSource: DB9PR10MB7100.EURPRD10.PROD.OUTLOOK.COM
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Jan 2024 08:03:13.2299
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: d350cf71-778d-4780-88f5-071a4cb1ed61
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Fl6jZOFU0v8N523q/FCXm6H0oGOtp9oYxkIHaDIL+8gSTJ0SLXtIWvbZx3VdDuS+JE/damo4XTQ3YT4FcNGLbWZIqUskjJBwxkHGdxNr7YY=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS5PR10MB8055

On 23/01/2024 01.26, Kees Cook wrote:
> For instances where only the overflow needs to be checked (and the sum
> isn't used), provide the new helper add_would_overflow(), which is
> a wrapper for check_add_overflow().
> 
> Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
> Cc: linux-hardening@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  include/linux/overflow.h | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/include/linux/overflow.h b/include/linux/overflow.h
> index 099f2e559aa8..ac088f73e0fd 100644
> --- a/include/linux/overflow.h
> +++ b/include/linux/overflow.h
> @@ -108,6 +108,22 @@ static inline bool __must_check __must_check_overflow(bool overflow)
>  		__builtin_add_overflow(__filter_integral(a), b,		\
>  				       __filter_ptrint(d))))
>  
> +/**
> + * add_would_overflow() - Check if an addition would overflow
> + * @a: first addend
> + * @b: second addend
> + *
> + * Returns true if the sum would overflow.
> + *
> + * To keep a copy of the sum when the addition doesn't overflow, use
> + * check_add_overflow() instead.
> + */
> +#define add_would_overflow(a, b)			\
> +	__must_check_overflow(({			\
> +		size_t __result;			\
> +		check_add_overflow(a, b, &__result);\
> +	}))

Hm, I think this is a bit too ill-defined. Why is the target type
hard-coded as size_t? What if a and b are u64, and we're on a 32 bit
target? Then a+b might not overflow but this helper would claim it did.

But we also cannot just use typeof(a+b) instead of size_t, since that
breaks when a and b are narrower than int (adding two u16 never
overflows since they get promoted to int, but then if assigning the
result to a u16 one truncates...).

Perhaps the target type must be explicit? sum_fits_in_type(T, a, b) ?
IDK, I just don't think size_t is the right thing to use in something
that is otherwise supposed to be type-generic.

Rasmus