Hi,
On 8/8/23 22:25, Jorge Lopez wrote:
> Hi Hans,
>
> On Mon, Aug 7, 2023 at 6:28 AM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
<snip>
>> 3. ordered_list_data->elements_size is set but never validated. You should compare elem after the loop with ordered_list_data->elements_size and make sure they match. IOW verify that 0-(ordered_list_data->elements_size-1) entries of the ordered_list_data->elements[] array have been filled.
>
> ordered_list_data->elements_size is checked against MAX_ELEMENTS_SIZE
> and not against the number of elements in the array. Initially, size
> value was reported (sysfs) but after a couple reviews, size was
> removed from being reported (sysfs). size value will be determined by
> the application when it enumerates the values reported in elements.
Right, but after splitting the string on commas there should be ordered_list_data->elements_size entries, right ? So we should verify that. Also what if the string after splitting has more entries then MAX_ELEMENTS_SIZE, then currently the code will overflow the array, so the loop splitting the string on commas should ensure that MAX_ELEMENTS_SIZE is not exceeded.
>>
>> 4. For specific values of eloc the code expects the current order_obj[elem] to be either an integer or a string, but this is not validated. Please validate that order_obj[elem].type matches with what is expected (string or int) for the current value of eloc.
>
> The purpose for 'eloc' is to help skip reading values such
> ORD_LIST_ELEMENTS and PREREQUISITES when ORD_LIST_ELEMENTS and/or
> PREREQUISITES_SIZE values are zero.
> Normally, 'eloc' and 'elem' are the same.
Never mind what I meant to say is that you should to a check like this:
/* Check that both expected and read object type match */
if (expected_order_types[eloc] != order_obj[elem].type) {
pr_err("Error expected type %d for elem %d, but got type %d instead\n"
expected_order_types[eloc], elem, order_obj[elem].type);
return -EIO;
}
But I see now that that check is already there.
Regards,
Hans
