Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <d89e76ab-4db8-9307-6c31-628df3b4fd94@intel.com>
Date:   Fri, 4 Aug 2023 19:33:33 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.13.0
Subject: Re: [PATCH net-next 1/3] virtchnl: fix fake 1-elem arrays in structs
 allocated as `nents + 1` - 1
Content-Language: en-US
To:     Kees Cook <keescook@chromium.org>
CC:     "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Larysa Zaremba <larysa.zaremba@intel.com>,
        "Andy Shevchenko" <andriy.shevchenko@linux.intel.com>,
        "Gustavo A. R. Silva" <gustavoars@kernel.org>,
        <netdev@vger.kernel.org>, <linux-hardening@vger.kernel.org>,
        <intel-wired-lan@lists.osuosl.org>, <linux-kernel@vger.kernel.org>
References: <20230728155207.10042-1-aleksander.lobakin@intel.com>
 <20230728155207.10042-2-aleksander.lobakin@intel.com>
 <202308040126.ADDA993@keescook>
 <49b0d7bd-2bc1-c994-7780-35554a399242@intel.com>
 <202308041029.74FF3CCD@keescook>
From:   Alexander Lobakin <aleksander.lobakin@intel.com>
In-Reply-To: <202308041029.74FF3CCD@keescook>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?WkxueEVBZnhZR3A1WWZWMlA2ZkEzajJZdHZSME1vcnZvWGFLb2pRYXJ4S29m?=
 =?utf-8?B?SzRiSzgrOFhSb1pCb1ZjVE15V0U1bFhOaDZtZ2wwQXE1RUdaR1BlaXNQZlVO?=
 =?utf-8?B?WWYvaDM1U3pER3dkTHVjTzlYT1QxOEJ1b0JYUzBxbVRxS0RvRVBhNnpTTjFG?=
 =?utf-8?B?OE1DTWpTRjgxNkkzUG5vem5zazNIY0lVM3IvYXVpTTVDKzhGWE1TZ2czQS9j?=
 =?utf-8?B?SzJZWDdLMDI5MTFadGwrMHkvVG03bXVFYUgrTkxBODdnWXR5dkRvUStDVHAw?=
 =?utf-8?B?VVdrNXZ2ejhuUHllVTVTdVBvUnhlVm5JRUpmNUhQUlhXaUhrRXBDL0R4dUt2?=
 =?utf-8?B?amVacjhkYUF4OFo0dk1vK095eU1vTGpTK0o0OHk4c2xxS3QwWnBINFZSbWJl?=
 =?utf-8?B?N1NyNnora1VnSnM3WTFXdHc1aXlsUXpVWGhnSjNBYnMvUzhIcWpXV3c0dC9h?=
 =?utf-8?B?RlBMSzIyVThVWThxSndicmhDTk1yc2xPZ1lKYlp6YklSZjJlRkRZdzg0bkdt?=
 =?utf-8?B?OVVKY3hWWkd1REtHRTRWTmRvbi8wQWJ0T2paeks1dGFDMGVZb3ZoREYrSXFx?=
 =?utf-8?B?TlQ1OXQySW5sUVZ4Z2llcE1KUThsYW0wN2Z6Njc2UGI3TnV5SFpoRG5na1Z0?=
 =?utf-8?B?Vkw3TTZtRXpBTFFUWTFWZEpRQzR5RW9MMDVOdGw3MVkxWmxuNndpN0Z0WXAx?=
 =?utf-8?B?UEhMODdGYndQeTJ4eWhhVU0zdDJ0bCtQZXorTW42LzdlbUI2TDYzOWZyN1FS?=
 =?utf-8?B?U1JVUzFFTlo5Q2J1RHdjSGo5QmtjSmQ4NDVhZTJ5K1NJL3liYVZMdUZ4eWg4?=
 =?utf-8?B?TDFKQXFvWW1nRTl4b1J2SzJtandRWmVRR0lCeWtlN2p1MzhxNTZlYkFCUjZW?=
 =?utf-8?B?dmI4K2hqekdWUllHaVA1bStlMm42dEdIdlZIWWN5RzNaQUZHalFYU3MxZjNm?=
 =?utf-8?B?TVpUS2xLdzd5UW5CSWlBS3IxT09yUmlNTGJwL0VPVWJKd2duakQ5UFlLSis2?=
 =?utf-8?B?aXM2ZUFybnJUOWNPVUpCYU9uamxqMVc4UHRmeCtTOXY0S0JDY2VVVzBDMnlZ?=
 =?utf-8?B?aEJ2Y1RXYllkK2RPcVZCQUlTaVRMRDg1TDFvWGlpVWxxcFBPWitnd1dLMUVC?=
 =?utf-8?B?NVloQzZzdEJlTVp0MWxtR2t6b0dWOEJWS3dQcFBMVWh2eDlFSUNudG1YMHJW?=
 =?utf-8?B?aFJ3T3JJcGs1V0EwUUs1R0FWTFRHaytRZ0Ridm1ZSDNkUGREcnlLNVhGTUdP?=
 =?utf-8?B?TURtaHhqR2dvOEozdlVXckRuY2FoYU5hNkNYeGR1SHlGRFhhYjdlOFVoeXpC?=
 =?utf-8?B?c0tCam1xQXI5M1hDdWU2VUswYlBDU0xKR0hUbldKWTB4MUFLODdwNW9CN09P?=
 =?utf-8?B?SFpjVmw4RDJadzVLMDAvbWt4ek93YVpqbjBvRmlxMCtjZEpCS1d4aUhYcDNp?=
 =?utf-8?B?ZUNlMityTEtXQjA5OFZ2VjY5WWJ2V1BBYzZDUS9KamFNTDZKUnU4Vm1XanV2?=
 =?utf-8?B?dnhXSXRNTlBIOVV1am5hSXBVRk10S3RNWGN1WGdNdzBma04xSHZWanlIeXpU?=
 =?utf-8?B?MURldkFCdTFnc3NWRUtWcEpRWGxoT0gwQjdkcitBSXhxVk1saFZXT1JTYkh4?=
 =?utf-8?B?akUxRHFHMVdQbzhiMzMreDVBQzJURVdUamhJcUZ4WTg4S3hiZC9zQm5QZkpi?=
 =?utf-8?B?TVNjVUc2cmhxZ3c5VmRiTFZzVm5wLzJnaXJHWGpvZlZ0R2NWcDFWYWl6d3po?=
 =?utf-8?B?NiswaGFhV0NEaVNra1psQXErb1o3Z0xmVHR5VTdRRnNoVEU1T2tWM3F1OFZX?=
 =?utf-8?B?OW5na0hoVzYxWDRxajlMVUdmcDBFOEYrekxJMTYwZTJYcFdCcXgwRWUwWW41?=
 =?utf-8?B?cTdRQ3RPYWk0VFkwNzNvbzRBbCtXK1EzUnVWT3AvcC80V3ZYbnVzRzgzVGdz?=
 =?utf-8?B?UXFNL2tEOFM0aHM2bEhZTW1uZWtKdUFCTHJxSzZKYitHQ2phSmRLZGxMUDZX?=
 =?utf-8?B?L01hYmFTeWhLREpQTVlpeUVmak9uSTVjemNnUEpUVTd5SVkxMVhSejN1NTdN?=
 =?utf-8?B?ZS9CbXQ4UUVXNU0rUHdndEh3VzNmaTVjcjdQRkYyY2lmMnNSSXByUEZ2eFZ3?=
 =?utf-8?B?aTExR09Qa1Y5eGNiT3dpZVB5VWNVdThpdlV2UjdWTkJNT2Zld0V1cDh1Qzh6?=
 =?utf-8?B?aXc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 8c257630-30f3-43cd-6f7d-08db9510f11e
X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB3625.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Aug 2023 17:33:40.3424
 (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: 8PG9S+bGXzaWTcg7mjGS+1K3qtr+O0jP/W3ppkd6PDwXMld+W/zkTC3pOs4/iUtSqDp7LmDWpwWWR8R4zZRrTuTWEk9HmctlHhscG4SLGp8=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB7572
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-2.2 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_BLOCKED,SPF_HELO_NONE,SPF_NONE,URIBL_BLOCKED
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

From: Kees Cook <keescook@chromium.org>
Date: Fri, 4 Aug 2023 10:29:48 -0700

> On Fri, Aug 04, 2023 at 05:42:19PM +0200, Alexander Lobakin wrote:
>> From: Kees Cook <keescook@chromium.org>
>> Date: Fri, 4 Aug 2023 01:27:02 -0700
>>
>>> On Fri, Jul 28, 2023 at 05:52:05PM +0200, Alexander Lobakin wrote:
>>>> The two most problematic virtchnl structures are virtchnl_rss_key and
>>>> virtchnl_rss_lut. Their "flex" arrays have the type of u8, thus, when
>>>> allocating / checking, the actual size is calculated as `sizeof +
>>>> nents - 1 byte`. But their sizeof() is not 1 byte larger than the size
>>>> of such structure with proper flex array, it's two bytes larger due to
>>>> the padding. That said, their size is always 1 byte larger unless
>>>> there are no tail elements -- then it's +2 bytes.
>>>> Add virtchnl_struct_size() macro which will handle this case (and later
>>>> other cases as well). Make its calling conv the same as we call
>>>> struct_size() to allow it to be drop-in, even though it's unlikely to
>>>> become possible to switch to generic API. The macro will calculate a
>>>> proper size of a structure with a flex array at the end, so that it
>>>> becomes transparent for the compilers, but add the difference from the
>>>> old values, so that the real size of sorta-ABI-messages doesn't change.
>>>> Use it on the allocation side in IAVF and the receiving side (defined
>>>> as static inline in virtchnl.h) for the mentioned two structures.
>>>>
>>>> Signed-off-by: Alexander Lobakin <aleksander.lobakin@intel.com>
>>>
>>> This is a novel approach to solving the ABI issues for a 1-elem
>>> conversion, but I have been convinced it's a workable approach here. :)
>>> Thanks for doing this conversion!
>>>
>>> Reviewed-by: Kees Cook <keescook@chromium.org>
>>>
>>
>> Thanks a lot!
>> You gave Reviewed-by for patches #1 and #3, does it mean the whole
>> series or something is wrong with the patch #2? :D
> 
> Hm, maybe delivery was delayed? I see it on lore:
> https://lore.kernel.org/all/202308040128.667940394B@keescook/
> 

Nevermind, my mail rules did put it in the folder other than the one
where the main thread was, sorry :s
Much thanks, I'm now a fan of _Generic() too :D

Olek