Hi Andrew, On Mon, Sep 03, 2018 at 10:05:54PM +0200, Andrew Lunn wrote: > > Just to be sure, we're talking here about making sure the value stored > > in the DT is not bigger than the specified value (here an u8)? If so, > > that isn't the reason why I'm suggesting those two patches. > > > > Without /bits 8/ in the DT property, whatever were the values I put in > > the property, I'd always get a 0. So I need to fix it either in the DT > > (but Rob does not really like it) or in the driver. > > Hi Quentin > > Ah, you are fixing endian issues. That was not clear to me from the > commit message. > > I don't know enough about how DT stores values in the blob. Is there > type info? Can the DT core tell if a value in the blob is a u8 or a > u32? It would be nice if it warned about reading a u8 from a u32 > blob. > From my quick research, the lower bound checking is performed by of_property_read_u* functions but not the higher bound checking (the internal function of_find_property_value_of_size allows higher bound checking but it seems it's never used by those functions (see 0 in sz_max of of_property_read_variable_u*_array)). sz_max is used by of_property_read_variable_u*_array to copy at a maximum of sz_max values in the output buffer. If sz_max is 0, it takes sz_min so it's an array of definite size. So since sz_max is 0 for all calls to of_property_read_variable_u*_array by of_property_read_u*_array, we basically know we'll get a buffer of sz_min values but we don't actually make use of the higher bound checking of of_find_property_value_of_size. We could enforce this higher bound check by, instead of setting sz_max to 0, setting sz_max to sz_min in calls to of_property_read_u*_array. But I guess there is a reason for sz_max being 0. Rob, Richard (commit signer of this code) do you know why? Could you explain? > Anyway, this change still removes some bounds checking. Are they > important? Do they need to be added back? > The edge-slowdown and the vddmac values are compared against a const array so we´re fine with those ones. For the led-X-mode, I added a constant for supported modes that gets checked when retrieving the DT property. So we´re fine here too. Quentin
Attachment:
signature.asc
Description: PGP signature
