linux-kernel - Re: [PATCH v3 07/14] software_node: Add support for fwnode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 24 Dec 2020 14:24:19 +0000
From:   Daniel Scally <djrscally@...il.com>
To:     Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc:     linux-kernel@...r.kernel.org, linux-acpi@...r.kernel.org,
        linux-media@...r.kernel.org, devel@...ica.org, rjw@...ysocki.net,
        lenb@...nel.org, gregkh@...uxfoundation.org, yong.zhi@...el.com,
        sakari.ailus@...ux.intel.com, bingbu.cao@...el.com,
        tian.shu.qiu@...el.com, mchehab@...nel.org, robert.moore@...el.com,
        erik.kaneda@...el.com, pmladek@...e.com, rostedt@...dmis.org,
        sergey.senozhatsky@...il.com, andriy.shevchenko@...ux.intel.com,
        linux@...musvillemoes.dk,
        laurent.pinchart+renesas@...asonboard.com,
        jacopo+renesas@...ndi.org, kieran.bingham+renesas@...asonboard.com,
        hverkuil-cisco@...all.nl, m.felsch@...gutronix.de,
        niklas.soderlund+renesas@...natech.se, slongerbeam@...il.com,
        heikki.krogerus@...ux.intel.com, linus.walleij@...aro.org
Subject: Re: [PATCH v3 07/14] software_node: Add support for fwnode_graph*()
 family of functions

On 24/12/2020 12:53, Laurent Pinchart wrote:
>> +	while ((port = software_node_get_next_child(parent, old))) {
>> +		/*
>> +		 * ports have naming style "port@n", so we search for children
>> +		 * that follow that convention (but without assuming anything
>> +		 * about the index number)
>> +		 */
>> +		if (!strncmp(to_swnode(port)->node->name, "port@",
>> +			     FWNODE_GRAPH_PORT_NAME_PREFIX_LEN))
> 
> I would either add a macro to replace the prefix ("port@"), or drop
> FWNODE_GRAPH_PORT_NAME_PREFIX_LEN. I think this is the worst of both
> worlds, the string and its length are defined in two different places
> :-)
> 
> I would personally drop the macro, but I don't mind either way as long
> as the string and its length are defined in the same place.

OK, pending outcome of the discussion in the other thread I'll do both
things the same way - whatever the decision there is.


>> +static int
>> +software_node_graph_parse_endpoint(const struct fwnode_handle *fwnode,
>> +				   struct fwnode_endpoint *endpoint)
>> +{
>> +	struct swnode *swnode = to_swnode(fwnode);
>> +	int ret;
>> +
>> +	/* Ports have naming style "port@n", we need to select the n */
>> +	ret = kstrtou32(swnode->parent->node->name + FWNODE_GRAPH_PORT_NAME_PREFIX_LEN,
>> +			10, &endpoint->port);
> 
> Same here.
> 
> I wonder if we should add a check to ensure parent->node->name is long
> enough (and possibly even start with the right prefix), as otherwise the
> pointer passed to kstrtou32() may be past the end of the string. Maybe
> this is overkill, if we can rely on the fact that software nodes have
> correct names.

Not necessarily actually; ports yes but endpoints no, so I think the
danger is there. I'll add the check.

> Reviewed-by: Laurent Pinchart <laurent.pinchart@...asonboard.com>

Thanks!