Received-SPF: pass (google.com: domain of linux-kernel+bounces-43451-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <8dead2fb-d522-492b-89f2-1358198c1cdf@prevas.dk>
Date: Mon, 29 Jan 2024 21:08:43 +0100
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 4/5] overflow: Introduce add_wrap(), sub_wrap(), and
 mul_wrap()
Content-Language: en-US, da
To: Kees Cook <keescook@chromium.org>
Cc: Mark Rutland <mark.rutland@arm.com>, linux-hardening@vger.kernel.org,
 "Gustavo A. R. Silva" <gustavoars@kernel.org>,
 Nathan Chancellor <nathan@kernel.org>, Bill Wendling <morbo@google.com>,
 Justin Stitt <justinstitt@google.com>, Miguel Ojeda <ojeda@kernel.org>,
 Marco Elver <elver@google.com>, linux-kernel@vger.kernel.org,
 llvm@lists.linux.dev
References: <20240129182845.work.694-kees@kernel.org>
 <20240129183411.3791340-4-keescook@chromium.org>
From: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
In-Reply-To: <20240129183411.3791340-4-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?d1ZtQVp3N1h6S2g5Y3dETjRWVW0zc1R3YlVBUFdCaHNxOGxkWi82bzV3Wmh5?=
 =?utf-8?B?WUs0bm42R0Vldi8xMno2cDJaUDMxQ1BCZ2xJR0tPdmJJbGJiYXgvRllGem8z?=
 =?utf-8?B?NU9Oa0ljUVBKdzNRdTI2MnBpbk5FVlpSVCtJazd2VTJQR081eUJlRnJqWkM4?=
 =?utf-8?B?UEdXa2dScEFLWFB4M1NpTTk5c1JEakJiSmpzL0ZRRmtDeEQyRDF3cW5RcVd2?=
 =?utf-8?B?TG9MbVFRSVRsRG9hbndJYTRRQkpoUWt5dVdSMHBGZEkzV1RValZ0ZmR1MG9Y?=
 =?utf-8?B?YXJJUlFkTFlQMUtueVYxL25EbFhuQVVHVStUY2VuQzc2ZzA1dHRvQkN0dStN?=
 =?utf-8?B?aFN3QkhERWN3L1doNWVYd0ZZakVKSlVRVmNOQ0hmSExoL1FqNDl2emx4dElh?=
 =?utf-8?B?cncrdFNjMkUzTEFSV0hDVmI2ellKSG5pNmVrWTFidWV3QURXa3RQalVTMndX?=
 =?utf-8?B?QmJRRWd0M0p4eVhLUVVzZ1FpVFd3d3NWUlVxczJFRjZsRjhzQ1NmeUpNUmx5?=
 =?utf-8?B?UVQydTNmVFhUeUdGRGc1VWw2R0o5aUg2MlBqa0pINnovejBvcDgwSy9uWjRq?=
 =?utf-8?B?aERxamhuTjVKZkZjd3Q5b3hVRnFNQWN3YXdtUUlIS1hlcmRYa0VveHdSRU8v?=
 =?utf-8?B?eE5BdE5WMGR3Q0tjRzBEMktnNTVVdW5NWkhVVUNFRlc4UVUzb3V2V2RUWDJC?=
 =?utf-8?B?TTJ1K0s1MU4raEJ6QWdENHJtZlczL2g2OCtxZWM5WkJVcVQxRW1YbVd5MndV?=
 =?utf-8?B?b0x1QUplSDhOYWw3Qlh2ZUVZSml4WUl6RmtIbldtN0ZxZHVPMXFtRGFaT3p4?=
 =?utf-8?B?UEZxdnl4MTk0R1NMRWczSytBcFgvL1dLYVpYZE5RWllsTnFnTEhua2R4cEtM?=
 =?utf-8?B?dmJubDBTZC9zYVpOREM3Z2FLMkRXMHFqejNHT0wxdFhjcjlKaUlHR2l3aDZI?=
 =?utf-8?B?bEkzd3U1aUI3ckd5S1lld0drc08zOFdXNzBYODZDVWpIZnZVdGJCejJGVUE3?=
 =?utf-8?B?Vm5lWWVUd3JqNkRPUm96SWRnNEs2cFFNQ0Yzek4rVy9ySjRRd29oYU9uM1pV?=
 =?utf-8?B?UVFrNkFhSmpMVkwzeFpTNVZvOU1QUkE4M3RuM2tDbUFSYmRPa291M0RRdkYy?=
 =?utf-8?B?MnZiZ0ZXK2plK2FONUthTHdNTDA5Nnh6UGhQSkhUR01hQjI5NUpSb0tTbmNE?=
 =?utf-8?B?eS9CdTgwY3FQdVFSZmJaNm5XS3p6YTdyVUp3YkpNRGlaZFdIR0toVEN1eXdZ?=
 =?utf-8?B?VVhBWkhQK0xwSnIrOVRLMzdtTEFnYjJFRDhTVityVE1yMTBOTnRNTEZmQW1M?=
 =?utf-8?B?MWs2TzBJbmtXU1kvandQdXZTUEU3NWdlbEJod1hGbzE0cXJlMDByWVovOWRI?=
 =?utf-8?B?L0I2dDMxWCtVTkh4bTlMbE1tQllVN1VaL3BxSG8va1hZMzVHdkp4TTh5Um5U?=
 =?utf-8?B?NGEweGROdmhZNVh1ZU1zQWQzaXJPbUFyazE2NG5seDZaTEplK3V5Y3BudlFq?=
 =?utf-8?B?UkVYRS9hWEk1UVZBNlpiS3lwdC9jNHN0aWc5cUdxdDd6eTdaN0NmOU5XMEVT?=
 =?utf-8?B?OW5FQTNZeUdVUlhsMFpvYkpDd0RxdUhSNXhOeGQzaUswQkhFTk5nb0FZQWxR?=
 =?utf-8?B?TlE5T1JnNGM1RzJCcEZMNklhc1FDMWxjY0sxN0hLS1ZtSVUrbDlWWjBBeEJP?=
 =?utf-8?B?RmhjTkxvYjdRb1FvOWtBWkRzYmVRbXdTWWx0Wmc4eTdwZXVFTnlJS2p5Uktw?=
 =?utf-8?B?ckxuYmZndXZ6WlRrNzE1blBBdXdaYTB6MmFBdS9qOXFzc05meGRsd0tyYWQ3?=
 =?utf-8?B?bVV1ZnhMYzNEcmNhM2loaHExd1JjSUluaGVKM3J6YWprL1Bab0V2UTRjdStC?=
 =?utf-8?B?UytLMW9IRkVaWkU5azFKcXJSclF4Z3JvWE5ESm5MQmFrU3pZUHRiaytUQlRr?=
 =?utf-8?B?Uml4Mjg4N1RTeVc1am5hOTVNYUw1Nlp2MVZpRnNjeXR0RmRtN2k4NFNCYTlB?=
 =?utf-8?B?QnVKR2V1aUxZdUltdnA5bU1aWnlpMjJzRXlVWU1xVW9FaW1vbHdDSzAwM2wr?=
 =?utf-8?B?RTk3U1FadDVEUFlSSVRJOEIyMDdLeE9xMzR0V1JEa0pYQ015dGVYOEN6V1dm?=
 =?utf-8?B?U0l3cUpVMzNHMlFDZEZ4SFRHQVgva0txYk95VmEzTHlIbzR5SXZEL0VtK2Vw?=
 =?utf-8?B?N3c9PQ==?=
X-OriginatorOrg: prevas.dk
X-MS-Exchange-CrossTenant-Network-Message-Id: 92dc2ec6-e89d-472e-1986-08dc210619a0
X-MS-Exchange-CrossTenant-AuthSource: DB9PR10MB7100.EURPRD10.PROD.OUTLOOK.COM
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 Jan 2024 20:08:46.4675
 (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: +yr43SU9WqicCZLSDa53vROMfOy5IuKSNvBG/cX8v+5UW8jk4XXVoDdQPaTTTP16iXVfMSql28MJ3Speb8yoALk8Sox9WXA7UGmXiWwHQe4=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM7PR10MB3159

On 29/01/2024 19.34, Kees Cook wrote:
> Provide helpers that will perform wrapping addition, subtraction, or
> multiplication without tripping the arithmetic wrap-around sanitizers. The
> first argument is the type under which the wrap-around should happen
> with. In other words, these two calls will get very different results:
> 
> 	add_wrap(int, 50, 50) == 2500
> 	add_wrap(u8,  50, 50) ==  196

s/add/mul/g I suppose.


> Cc: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: linux-hardening@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  include/linux/overflow.h | 54 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 54 insertions(+)
> 
> diff --git a/include/linux/overflow.h b/include/linux/overflow.h
> index 3c46c648d2e8..4f945e9e7881 100644
> --- a/include/linux/overflow.h
> +++ b/include/linux/overflow.h
> @@ -120,6 +120,24 @@ static inline bool __must_check __must_check_overflow(bool overflow)
>  		check_add_overflow(var, offset, &__result);	\
>  	}))
>  
> +/**
> + * add_wrap() - Intentionally perform a wrapping addition
> + * @type: type to check overflow against

Well, nothing is "checked", so why not just say "type of result"?

>  
> +/**
> + * sub_wrap() - Intentionally perform a wrapping subtraction
> + * @type: type to check underflow against

The terminology becomes muddy, is (INT_MAX) - (-1) an underflow or
overflow? Anyway, see above.

>  
> +/**
> + * mul_wrap() - Intentionally perform a wrapping multiplication
> + * @type: type to check underflow against

And here there's definitely a copy-pasto.

The code itself looks fine.

Rasmus