On Thu, Aug 24, 2017 at 12:23 PM, Rob Herring <robh@xxxxxxxxxx> wrote:
> On Wed, Aug 23, 2017 at 9:03 PM, David Gibson
> <david@xxxxxxxxxxxxxxxxxxxxx> wrote:
>> On Tue, Aug 22, 2017 at 06:02:06PM -0500, Rob Herring wrote:
>>> Many common bindings follow the same pattern of client properties
>>> containing a phandle and N arg cells where N is defined in the provider
>>> with a '#<specifier>-cells' property such as:
[...]
>>> + if (prop->val.len < ((cell + cellsize + 1) * sizeof(cell_t))) {
>>> + FAIL(c, dti, "%s property size (%d) too small for cell size %d in %s",
>>> + prop->name, prop->val.len, cellsize, node->fullpath);
>>> + }
>>
>> How will this cope if the property is really badly formed - e.g. a 3
>> byte property, so you can't even grab a whole first phandle? I think
>> it will trip the assert() in propval_cell_n() which isn't great.
>
> At least for your example, we'd exit the loop (cell < 3/4). But I need
> to a check for that because it would be silent currently. I'll add a
> check that the size is a multiple of 4 and greater than 0.
>
> However, the check here is not perfect because we could have
> "<&phandle1 1 2>" when really it should be "<&phandle1 &phandle2
> &phandle3>". We don't fail until we're checking the 2nd phandle.
> That's why I added the "or bad phandle" and the cell # in the message
> above. In the opposite case, we'd be silent. One thing that could be
> done is double check things against the markers if they are present.
Here's what that looks like:
/* If we have markers, verify the current cell is a phandle */
if (prop->val.markers) {
struct marker *m = prop->val.markers;
for_each_marker_of_type(m, REF_PHANDLE) {
if (m->offset == (cell * sizeof(cell_t)))
break;
}
if (!m)
FAIL(c, dti, "Property '%s', cell %d is not a valid phandle in %s",
prop->name, cell, node->fullpath);
}
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
