On 09/22/16 11:33, SF Markus Elfring wrote: > From: Markus Elfring <elfring@xxxxxxxxxxxxxxxxxxxxx> > Date: Thu, 22 Sep 2016 10:06:50 +0200 > > The of_node_put() function was called in some cases > by the tilcdc_convert_slave_node() function during error handling > even if the passed variable contained a null pointer. > > * Adjust jump targets according to the Linux coding style convention. > > * Split a condition check for resource detection failures so that > each pointer from these function calls will be checked immediately. > > See also background information: > Topic "CWE-754: Improper check for unusual or exceptional conditions" > Link: https://cwe.mitre.org/data/definitions/754.html > I don't really agree with this patch. There is no harm in calling of_node_put() with NULL as an argument and because of that there is no point in making the function more complex and harder to maintain. Best regards, Jyri > Signed-off-by: Markus Elfring <elfring@xxxxxxxxxxxxxxxxxxxxx> > --- > drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c | 28 +++++++++++++++++----------- > 1 file changed, 17 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c b/drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c > index 6204405..6ee5865 100644 > --- a/drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c > +++ b/drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c > @@ -209,25 +209,27 @@ void __init tilcdc_convert_slave_node(void) > return; > > lcdc = of_find_matching_node(NULL, tilcdc_of_match); > - slave = of_find_matching_node(NULL, tilcdc_slave_of_match); > + if (!of_device_is_available(lcdc)) > + goto free_table; > > - if (!slave || !of_device_is_available(lcdc)) > - goto out; > + slave = of_find_matching_node(NULL, tilcdc_slave_of_match); > + if (!slave) > + goto put_node_lcdc; > > i2c = of_parse_phandle(slave, "i2c", 0); > if (!i2c) { > pr_err("%s: Can't find i2c node trough phandle\n", __func__); > - goto out; > + goto put_node_slave; > } > > overlay = tilcdc_get_overlay(&kft); > if (!overlay) > - goto out; > + goto put_node_i2c; > > encoder = of_find_matching_node(overlay, tilcdc_tda998x_of_match); > if (!encoder) { > pr_err("%s: Failed to find tda998x node\n", __func__); > - goto out; > + goto put_node_i2c; > } > > tilcdc_copy_props(slave, encoder, tilcdc_slave_props, &kft); > @@ -238,10 +240,10 @@ void __init tilcdc_convert_slave_node(void) > continue; > if (!strncmp("i2c", (char *)prop->value, prop->length)) > if (tilcdc_prop_str_update(prop, i2c->full_name, &kft)) > - goto out; > + goto put_node_fragment; > if (!strncmp("lcdc", (char *)prop->value, prop->length)) > if (tilcdc_prop_str_update(prop, lcdc->full_name, &kft)) > - goto out; > + goto put_node_fragment; > } > > tilcdc_node_disable(slave); > @@ -252,12 +254,16 @@ void __init tilcdc_convert_slave_node(void) > else > pr_info("%s: ti,tilcdc,slave node successfully converted\n", > __func__); > -out: > - kfree_table_free(&kft); > + put_node_fragment: > + of_node_put(fragment); > + put_node_i2c: > of_node_put(i2c); > + put_node_slave: > of_node_put(slave); > + put_node_lcdc: > of_node_put(lcdc); > - of_node_put(fragment); > + free_table: > + kfree_table_free(&kft); > } > > int __init tilcdc_slave_compat_init(void) > -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
