| Message ID | 20190318055219.19029-3-stewart@linux.ibm.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/3] hdata: Add protection against corrupt ntuples structure | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch master (2ba5ce84a197ee61423355f443a3ff3eea185ff1) |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot | success | Test snowpatch/job/snowpatch-skiboot on branch master |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot-dco | success | Signed-off-by present |
On 03/18/2019 11:22 AM, Stewart Smith wrote:
> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
Patch looks good.
I see few other places inside iohub.c where we don't check dt_new* return value.
May be fix them as well?
-Vasant
Vasant Hegde <hegdevasant@linux.vnet.ibm.com> writes: > On 03/18/2019 11:22 AM, Stewart Smith wrote: >> Signed-off-by: Stewart Smith <stewart@linux.ibm.com> > > Patch looks good. > > I see few other places inside iohub.c where we don't check dt_new* return value. > May be fix them as well? Huh, we probably should. afl-fuzz is pretty good at crashing things here, and I have mixed feelings about how much effort to put into it...
On 03/20/2019 12:03 PM, Stewart Smith wrote: > Vasant Hegde <hegdevasant@linux.vnet.ibm.com> writes: >> On 03/18/2019 11:22 AM, Stewart Smith wrote: >>> Signed-off-by: Stewart Smith <stewart@linux.ibm.com> >> >> Patch looks good. >> >> I see few other places inside iohub.c where we don't check dt_new* return value. >> May be fix them as well? > > Huh, we probably should. afl-fuzz is pretty good at crashing things > here, and I have mixed feelings about how much effort to put into it... > May be we can add assert() inside dt_new* functions. If we fail to allocate memory for device tree then we have bigger problem anyway. -Vasant
Vasant Hegde <hegdevasant@linux.vnet.ibm.com> writes: > On 03/20/2019 12:03 PM, Stewart Smith wrote: >> Vasant Hegde <hegdevasant@linux.vnet.ibm.com> writes: >>> On 03/18/2019 11:22 AM, Stewart Smith wrote: >>>> Signed-off-by: Stewart Smith <stewart@linux.ibm.com> >>> >>> Patch looks good. >>> >>> I see few other places inside iohub.c where we don't check dt_new* return value. >>> May be fix them as well? >> >> Huh, we probably should. afl-fuzz is pretty good at crashing things >> here, and I have mixed feelings about how much effort to put into it... >> > > May be we can add assert() inside dt_new* functions. If we fail to allocate memory > for device tree then we have bigger problem anyway. Although that would be it in the PCI hotplug code path, and being unable to process the new device shouldn't be a fatal error.
diff --git a/hdata/iohub.c b/hdata/iohub.c index ad1ddae401f0..028fc6abdc0f 100644 --- a/hdata/iohub.c +++ b/hdata/iohub.c @@ -660,6 +660,10 @@ static void parse_one_slot(const struct slot_map_entry *entry, case st_rc_slot: node = dt_new_2addr(dt_slots, "root-complex", chip_id, entry->phb_index); + if (!node) { + SM_ERR("Couldn't add DT node\n"); + return; + } dt_add_property_cells(node, "reg", chip_id, entry->phb_index); dt_add_property_cells(node, "#address-cells", 2); dt_add_property_cells(node, "#size-cells", 0);
Signed-off-by: Stewart Smith <stewart@linux.ibm.com> --- hdata/iohub.c | 4 ++++ 1 file changed, 4 insertions(+)