On Wed, Nov 06, 2024 at 09:54:33AM +0100, Janosch Frank wrote: > On 11/6/24 9:10 AM, Heiko Carstens wrote: > > On Mon, Nov 04, 2024 at 04:36:09PM +0100, Steffen Eiden wrote: > > > + copy_len = sizeof(list->secrets[0]) * list->num_secr_stored; > > > + WARN_ON(copy_len > sizeof(list->secrets)); > > > > Is this really possible? Without checking the documentation I guess > > this is not possible and therefore the WARN_ON() should be removed. > > > > This happening requires a FW error, no? > list->num_secr_stored is reported by FW and would need to be >85. > > We could clamp it down to 85 secrets / 4k - sizeof(header) with a > WARN_ON_ONCE() to catch FW problems if that suits you more. If this would be an *error* why even add this check? We have tons of code without doing sanity checks for firmware provided values - where should we start or end? So imho: either remove this check if this would be firmware error, unless there is a good reason do keep this check, or if this is not an error convert to WARN_ON_ONCE() and limit the copy_to_user().
