Received-SPF: pass (google.com: domain of linux-kernel+bounces-73727-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <787dcf27-c434-42e0-9c80-35e341aa16c4@intel.com>
Date: Tue, 20 Feb 2024 13:27:45 -0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH -next] mux: convert mux_chip->mux to flexible array
Content-Language: en-US
To: Kees Cook <keescook@chromium.org>, Jesse Brandeburg
	<jesse.brandeburg@intel.com>
CC: Peter Rosin <peda@axentia.se>, <gustavoars@kernel.org>,
	<linux-kernel@vger.kernel.org>, <linux-i2c@vger.kernel.org>
References: <20230223014221.1710307-1-jacob.e.keller@intel.com>
 <e8782296-49bc-33a2-47b3-45c204551806@intel.com>
 <202402182100.1D5BBE45@keescook>
From: Jacob Keller <jacob.e.keller@intel.com>
In-Reply-To: <202402182100.1D5BBE45@keescook>
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?REZseXBidjdEbEkwRGh2ODluRkdremtSTWFEOU9yMnlXNFZ1bGRNTCsxUFlP?=
 =?utf-8?B?U3ptVmJwMkpJbE1DdjZDWnVKUWp2WndBdXB6bVVxWVhUdDNaS0czNmhPSTRp?=
 =?utf-8?B?aUgzNDJaZkczeEx5K0Y5L0ZNRDJ0R2NYb3p1T1Z2WGR0OXFmQUZHcVFwV0I1?=
 =?utf-8?B?T2syeGtSKzJHVnpJeDZYUEpmNzFiRnFRakgwRFZqei91SktwanVpMmFYMVFm?=
 =?utf-8?B?c0M5Uk8vOVdrREJ0R3V6Vmk1UitZUkVLaEZYazJlRnJsNVVFL1BRRks3Qmlx?=
 =?utf-8?B?TGlpUlVtYmd2eUZLSXVGNld6NTdkM3AvL1A0UUtUWUNxZkZLQ0VueGFLQ1c1?=
 =?utf-8?B?TzJHeFRCcFI5VUdnOEVGRlZEVVdlekx0dVMzZzk0cnRtelJ5OCtnRG94NzlC?=
 =?utf-8?B?RFNPOW5QR3dIZWVpNHkvSlVmemJKT1dFTEdVbjgzeWdPT2psTUJsVTlscmky?=
 =?utf-8?B?cTlGaFREV1huSlhvZ2c1ZDZDQk44QUI5MENsNE9YeFJMVzVEVVpKV0FEL2p0?=
 =?utf-8?B?aG5pZ21FTGFINDVDNFQrNzBYcm4xVk45Q1ZmeXFBWlR5bUhabmpQY25hMlY4?=
 =?utf-8?B?OEVGMjNxUXRQZmgvUzJrZmRrcllQTXNmSkRBRmVKcnRvNFZjVDQ4WkJJWERM?=
 =?utf-8?B?UVpRRG5JenZQdUJmOGc2dEF6b3pzYkVtUnVTcjdVWURvRWRLOUdHN3IrbFMz?=
 =?utf-8?B?WHRCTXExN00vZ1BpbFNLVkJnay8rMDlGVXo0S3BpYXRUTUU1WGNvdGtUTnM4?=
 =?utf-8?B?MmdqdzBmamFZVGlpZk43RStmM0xYQmFiR21pNEJ5cnR0UmhNeVUvNUhDNlli?=
 =?utf-8?B?VFdzREhJemFwMVhTK3hCSmNVNWZjZFQ2UktyWjBZbW00L0wzemRmcUdRcFV1?=
 =?utf-8?B?ZU5EL2VQbGljeE9uLzk5aVlHSmN6QUFIajNzVm51cVZYcW1ndmZhMXNDMXlL?=
 =?utf-8?B?QUJlNzR5TE5nMWlTbVJ3ZEFkQ1lPVTV0ZWRVaE1mMFA0bzZsNEJ1TXZodHcw?=
 =?utf-8?B?L1A4RnJjZWhtak1QWTJ0VHd5akx3MVdDUFRCQ3UwQ09zQVhVWVB2MSttS0lk?=
 =?utf-8?B?SDFJZWdTaXpQREdQQ3VrWnZjanNoMTJxdy9TaEZwN20xTDUyRWU0S3lDVUV4?=
 =?utf-8?B?NGU3dnZFTzZOaEdrcFVJZDVwKy9tc3ZuUTdmdENVSTFtUDNRRURZa3NsRVhX?=
 =?utf-8?B?dm4rakpxMFk1VThJZnNlUjNaelNEL0lYdkpwR3lXZVFDWXlyVm5LOURFZ2VO?=
 =?utf-8?B?bUNYcVJFVnVoU0haRnM0M29wT3ZkQWNTVTRQWkhTVkhoa01Fdk5SWEk5MFgw?=
 =?utf-8?B?ZEpja1I4b2EzeFFseGtLeklPOUpXbGxGcXJaS2VrL1ozRnJmS3dPWlZHUisw?=
 =?utf-8?B?SFovaHFXUkhtKytzV3VIMkFzbWtRUmVZVExqVDhVa2VLekF0RHI1WXVLdm11?=
 =?utf-8?B?MjEvNkQ3U2NTK3o4eG92NktJS1djUGxlQS9aSWZBR05zUzNtSTZVNE1JMVNq?=
 =?utf-8?B?aE1FL2htZWhXQlNTdStRZkZZbUZEZ1hNTGQrZzZqUHNrdFF6NFRHcm4vRmhv?=
 =?utf-8?B?U2NoZFpWK0cvdzhTZUVzakhlNEJ5TTJJUlRGOFo2VW5hYVB6TS81b2wrRm1F?=
 =?utf-8?B?VFRJSklCdm5lYVRoUnNyK2g0ZkFMT1FSbzI5N2tTRUtWb0k5YWF5TmdFdU8y?=
 =?utf-8?B?QUJMOUw2T0dFY0tHUWp5RG5NK0pWZmtHUVRvejRnVElvdVhKaUFBNnF5RE9p?=
 =?utf-8?B?aElLcGc5dnkvRzdvZW5TeUJjNEl1WldJTVE0aEM2bDVEeXg2MkI1U2xYQlRu?=
 =?utf-8?B?UDdIV2pEYWdWQ1FrdVZxNFkvcmJyaDlvYnpwUlVIUm9Rd1ZjL2dzV2JQN2JE?=
 =?utf-8?B?VHhoNW5vVnR4OXVZRStPVENwam1OTEgwcEJuSVp1dnZYQWJHMkl4L0JDck16?=
 =?utf-8?B?UkJRTHpON05OZlFrVG1lU3IrOGp6a3A5WjdHQzFzYVN2bFZ4elVDUm44SERw?=
 =?utf-8?B?VTh2YUtJYzNSbjh6aWFVNUMvQU1BMHFCQTRmODZGSmxpZkdwMGtCUmg0Q0J6?=
 =?utf-8?B?ZTMydHZ6WUg1QllZalhYbnNPWVFkT0YzN0xORVFFdmJCWEIzYXY4M2FZWXhE?=
 =?utf-8?B?SzA2UjR6TkdOaFFPTzlleWdRNk16Slo4TmgrNVpLSnhTK25RTVlRTkpQU3cz?=
 =?utf-8?B?aWc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 42767070-9186-45d8-255e-08dc325ac86d
X-MS-Exchange-CrossTenant-AuthSource: CO1PR11MB5089.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Feb 2024 21:27:47.1696
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: SRLyPuggU07XXen2NqHfGyLMQ2rUBxqjpxrwv9j3viOonzlM0kf0i/siHkTCP9dwWxLe3eLE+LKJEbi8WOZf58TQggyAXBdXxFoJjsrsTaw=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH3PR11MB7867
X-OriginatorOrg: intel.com


On 2/18/2024 9:04 PM, Kees Cook wrote:
> On Mon, Feb 27, 2023 at 12:28:43PM -0800, Jesse Brandeburg wrote:
>> On 2/22/2023 5:42 PM, Jacob Keller wrote:
>>> The mux_chip structure size is over allocated to additionally include both
>>> the array of mux controllers as well as a device specific private area.
>>> The controllers array is then pointed to by assigning mux_chip->mux to the
>>> first block of extra memory, while the private area is extracted via
>>> mux_chip_priv() and points to the area just after the controllers.
>>>
>>> The size of the mux_chip allocation uses direct multiplication and addition
>>> rather than the <linux/overflow.h> helpers. In addition, the mux_chip->mux
>>> struct member wastes space by having to store the pointer as part of the
>>> structures.
>>>
>>> Convert struct mux_chip to use a flexible array member for the mux
>>> controller array. Use struct_size() and size_add() to compute the size of
>>> the structure while protecting against overflow.
>>>
>>> After converting the mux pointer, notice that two 4-byte holes remain in
>>> the structure layout due to the alignment requirements for the dev
>>> sub-structure and the ops pointer.
>>>
>>> These can be easily fixed through re-ordering the id field to the 4-byte
>>> hole just after the controllers member.
>>
>> Looks good to me (just a driver dev, not a mux dev!). Also added
>> linux-i2c mailing list and a couple others for more review.
>>
>> Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
>>
>> related thread (cocci script) at [1]
>>
>> [1]
>> https://lore.kernel.org/all/20230227202428.3657443-1-jacob.e.keller@intel.com/
> 
> *thread necromancy*
> 
> Can we land this? It's the last struct_size() instance that the above
> Coccinelle script flags.
> 
> Reviewed-by: Kees Cook <keescook@chromium.org>
> 

I'm happy to send a v2 if we need.

Thanks,
Jake