Thanks for the review. Both findings are addressed in v3.
- [Medium] Incomplete fix in i40e_ddp_does_profile_exist() and i40e_ddp_does_profile_overlap() Agreed on the clamping half. Silently limiting the count and then returning 0 hands i40e_ddp_load() a definite answer derived from a list that was only partially read: the add path proceeds without having examined every profile the firmware reported, and the is_add == false path reports a profile as missing that may in fact be loaded. Both helpers already have an error return that i40e_ddp_load() turns into "Failed to fetch loaded profiles." and aborts the operation, so v3 rejects the list instead: if the firmware reports more profiles than I40E_PROFILE_LIST_SIZE can hold, the helpers return -EIO and no scan happens at all. I did not plumb the response datalen out of i40e_aq_get_ddp_list(). For this command the driver has to set desc.datalen to the buffer size it passes in, and i40e_aq_get_ddp_list() does not report the writeback descriptor back to its caller, so the length would have to come either from a new output parameter on that helper (declared in i40e_prototype.h) or from a wb_desc routed through cmd_details. Either way it adds a second firmware supplied number to validate on top of the one this patch is about, and if the firmware leaves the field at the requested 772 bytes the derived bound is exactly the bound we have today. That is a bigger change to the common AQ path than a net fix should carry, so I would rather bound the scan by the size of the buffer the driver owns. The remaining half - comparing against p_info[] slots the firmware never filled - cannot be detected independently of the count, because the driver is not told how many records were actually written. If the firmware reports a count of 16 or less while writing fewer records, the unread slots are indistinguishable from real entries. Zero initialization makes the outcome deterministic (an unwritten entry is all zeroes rather than stale stack) but does not make it correct; deriving the bound from the response length would be needed for that, and that is the larger helper change described above. - [Medium, pre-existing] uninitialized buff[] handed to i40e_aq_get_ddp_list() Agreed, and fixed as suggested: buff[] is now zero initialized in both helpers. i40e_asq_send_command_atomic_exec() copies the full buff_size into the DMA bounce buffer and copies the full buff_size back on completion, so those 772 bytes of stack were both readable by the firmware and compared against afterwards. Both hunks touch these declarations and the root commit is the same (cdc594e00370), so it is handled here rather than as a separate patch; I can split it out if you would rather have it on its own. v3 keeps the unsigned loop counter from v2, and drops the Reviewed-by tag, as the code changed after the review. pw-bot: cr
