Message-ID: <43543fd7-1cd0-ee33-5a8e-d9d3c7d0e257@intel.com>
Date:   Tue, 28 Feb 2023 09:48:50 -0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.6.1
Subject: Re: [PATCH 1/3] wifi: ipw2x00: convert ipw_fw_error->elem to flexible
 array[]
Content-Language: en-US
To:     Johannes Berg <johannes@sipsolutions.net>
CC:     Kalle Valo <kvalo@kernel.org>, <linux-wireless@vger.kernel.org>,
        Stanislav Yakovlev <stas.yakovlev@gmail.com>
References: <20230228162827.3876606-1-jacob.e.keller@intel.com>
 <a8798dce4ae87aee64dfd56721b1668f8c969951.camel@sipsolutions.net>
 <d393ba90-ecdd-ffea-540b-d6db15571d5b@intel.com>
 <7f996d2efd23cefd17074edaeed0a6bbbd9f1a99.camel@sipsolutions.net>
From:   Jacob Keller <jacob.e.keller@intel.com>
In-Reply-To: <7f996d2efd23cefd17074edaeed0a6bbbd9f1a99.camel@sipsolutions.net>
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?QUIrZzBBQXR3YW9qSDA3RmhqL2hjNERqK090MEhZZlhMWEM0RGVPbDJwZWUy?=
 =?utf-8?B?T3Y3M0RlK2o1b1ZIa3NPVTRicldRQVFpeDRuYnVxVVV6Nk5TVFhVSTBCSWF1?=
 =?utf-8?B?eVUwaG8zcWJuMzdtb3U1VVd0ZzZDWXhLMkVpNytJdHkxSFhwRlM2VUU5emEx?=
 =?utf-8?B?NU81SDRPTXNOb3pRYWFBNzllSHpvTmxnRWUrUVNUdnZXN1R4WGF0L1VXSVE0?=
 =?utf-8?B?ZjFLUFhnQ3JRWXZSem9CbzBVTUg0NTJNZ0FlWmg5NTROY0N6ODBWY3dpa3Fv?=
 =?utf-8?B?OCtEdTNVYzJ0V2dJckgwakpndENMbUxNWGo3RkVaeDNPRDd2VGN3eHhyL2xX?=
 =?utf-8?B?NDRwVE1iS2pWb1JvRUFtMzRpcktlQzNEdkZNeEhueG5KMVltYWt1aHdvMUlu?=
 =?utf-8?B?a1lXZEJPaS92b2t6czViUlZKQkpDdndrRU9CTVM4bHgwSHZJZXRIa2I5T1dL?=
 =?utf-8?B?TEwwbWU2K1E3Z1Bmb2haUjQ2K2tpS0k4YXRyZnlNeHNwa2JsWWhZNTBhdm8x?=
 =?utf-8?B?QWdtcTJ2S0ZxU0dYdUhreDlTK1Z6M2xmZ2tIc3Fvd0J0YnVVd1FGSFZ1YnFS?=
 =?utf-8?B?SE5Jbld0YVQ2SU5NZHZITDZpMUdmYWlOSEphSk1BSnZrazVMVmRueXc0Yk1O?=
 =?utf-8?B?SE1qUVViZkxBQkFES1ZjUGhoRUNoWmpVSFJ4RUxya1NyK1BQWWVEZmZESS9n?=
 =?utf-8?B?Y1ZjM3VXNFFna0gxQUdIaVNhZXNuOE0vZG10amdnZVR1ellVSGxsbXdZREtr?=
 =?utf-8?B?c2p3bkp2SytOMVc0QThmUUpYWjE3WjZrVXZIaHZBd0VoUFVsbjZsdWkwNlRB?=
 =?utf-8?B?S0xYanIyS25jdnRqLzloUUNCek93dGtObDgrWm9rSHEzZUwzWEw4VCtPUDVH?=
 =?utf-8?B?RXZDWlFuVnZnWVpYTzBNQm1ITEhtL05UUlF3eUhMSk0wdUJ5N2xmUzFaNHZt?=
 =?utf-8?B?ZkkwQ3FOQXk4NjQwOTdsVWNxa212aXZ3QXdZMWpDYzFTU2Z2Z3YzTU9uR3R4?=
 =?utf-8?B?TXAvcHJhaDNYWXlDbUMwK3hIL0l1Sk9wRGo3dDFUZXRtSU5pY3ozN0ZoZXhr?=
 =?utf-8?B?eXdROTRVV2JYQ3VMVUlWUEQxZHc2ZFRqT0JWeFlXWjMrOXN1M2JJTjVJODIw?=
 =?utf-8?B?cUdqdzg3dUJJRCs3cWROa0lJdmF3R1I4YVNaUm0zU2RRZ2FpUG1JL2Z0SUNF?=
 =?utf-8?B?TjFORkpsQVowc3V4UmhQcElTZlJOTkFLZ2tnZTdZSUw4NDJkVDVPUEpKbW1M?=
 =?utf-8?B?SE9tcmU5WFlwZ3NIaFFuVisyN2hXSWppbjllL3lha0VSMDh3dTdydXU5Ulcy?=
 =?utf-8?B?S3RSWjVjdGV2OFYxckV3Z21iaG9wc29nRmVNUndvY3RmNUU1VVB3RnJYNFc0?=
 =?utf-8?B?MHRPQTA5Q3drb3YxbU9jWlRpWmVEc3NiTGlUeE04SmlNR0t3TEJ1bnFMZjhH?=
 =?utf-8?B?T2NIK3pPb0dEUmtEdnNJTE5mL01OcHhZU3hLekJxNG5Pd0FuNXdhemgva2RF?=
 =?utf-8?B?YzRYd1JIYXZNK0lqZ3RxSXhObTR3R09ua2l4bGZ3cERENzUybUJBZGtvT2ta?=
 =?utf-8?B?dktQV25GUXVmeVI0d1BFSHJ3TityWnBFL1dGcVI1YVpFVEIrUXFTYU93TURE?=
 =?utf-8?B?RWxRQlczN1NBc2VNamZKREVCVkN2SnorWlUzWitrU3FETDZiVmZjVm9ZdXp6?=
 =?utf-8?B?R3hwZzNuNVVrZTJ3U0tjSzZmT013Nk1sc01ORzNqRUw4cmtLWTJ5Ty9RR28x?=
 =?utf-8?B?UEVtSE1lYUhyVkU1YTVTNUJCZ1B4QVFiKy8xK1BEN1pnWGxjcmxlVWxZbit1?=
 =?utf-8?B?YWpPcHNZR2ZxTkxqTUJoRlNBZy9PTGtiVndGTzlYVk9hSGROMW9DVWNLQUhI?=
 =?utf-8?B?TGM2UHZaUjZlMVFWb1cxVDJNRXBTTDc4aVFOUFRvUERwZ08vQ1pwUHJ3SWgy?=
 =?utf-8?B?L3d3RHR1Mk00RTlXRUFWS2tnZUIzcmM3NDNRYlNyTGlUUUEyK0tib1RRSGxr?=
 =?utf-8?B?Nk9mUWIrV1pqaDlrSStEQjdnZkpsaWRZb1EyUGlEWklqL0tQeEtzbUlnMTgz?=
 =?utf-8?B?TEFWZWZmWGE1aE04NkFmdTd2STdzb290ZFJMN1R2dllDQ1ZmdzdNRUFTVHd4?=
 =?utf-8?B?OWVUQit5aitkbndQZFJQeXJkZnVjSG55UlNQVFJXQ1BzRkRsREZqZ1p3MkRH?=
 =?utf-8?B?SEE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 67e319a3-280b-42ed-7d82-08db19b40e48
X-MS-Exchange-CrossTenant-AuthSource: CO1PR11MB5089.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Feb 2023 17:48:53.0152
 (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: KiH3OnUX0KIep84gXDrX+HgQjadatzRAWxG1CbFKGRTf++vlwgy+k7AIDg6QPVk2OXgAV1963b5oSgRTr/fi0K9eZ3pNjiSxp4PO8fYG//8=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS7PR11MB7859
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-wireless.vger.kernel.org>
X-Mailing-List: linux-wireless@vger.kernel.org


On 2/28/2023 9:46 AM, Johannes Berg wrote:
> On Tue, 2023-02-28 at 09:44 -0800, Jacob Keller wrote:
>>
>> Previous to this change, error struct has two pointers to sections of
>> memory allocated at the end of the buffer.
>>
>> The code used to be:
>>
>> -	error = kmalloc(sizeof(*error) +
>> -			sizeof(*error->elem) * elem_len +
>> -			sizeof(*error->log) * log_len, GFP_ATOMIC);
>>
>> i.e. the elem_len is multiplying sizeof(*error->elem).
>>
>> The code is essentially trying to get two flexible arrays in the same
>> allocation, and its a bit messy to do that. I don't see how elem_len
>> could be anything other than "number of elems" given this code I removed.
> 
> Yeah, you're right. I was thinking of more modern HW/FW too much I
> guess, I see now even in the driver we have an array walk here (and it
> trusts the elem_len from firmware... ahrg!)
> 

Ouch.. that makes me feel better about using struct_size/size_add here
since it would help protect against an overflow with a large element size...