linux-kernel - Re: [RFC PATCH] i2c: skip of_i2c_register

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 linux-cve-announce 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]

Message-ID: <92217ec6-c21c-462a-a934-9e93183c1230@quicinc.com>
Date: Thu, 31 Oct 2024 11:45:53 -0700
From: Abhinav Kumar <quic_abhinavk@...cinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
CC: Wolfram Sang <wsa+renesas@...g-engineering.com>,
        <freedreno@...ts.freedesktop.org>, <dri-devel@...ts.freedesktop.org>,
        <robdclark@...il.com>, <swboyd@...omium.org>, <airlied@...il.com>,
        <quic_jesszhan@...cinc.com>, <lyude@...hat.com>, <simona@...ll.ch>,
        <linux-i2c@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH] i2c: skip of_i2c_register_device() for invalid child
 nodes



On 10/31/2024 11:23 AM, Dmitry Baryshkov wrote:
> On Wed, 30 Oct 2024 at 03:07, Abhinav Kumar <quic_abhinavk@...cinc.com> wrote:
>>
>> of_i2c_register_devices() adds all child nodes of a given i2c bus
>> however in certain device trees of_alias_from_compatible() and
>> of_property_read_u32() can fail as the child nodes of the device
>> might not be valid i2c client devices. One such example is the
>> i2c aux device for the DRM MST toplogy manager which uses the
>> display controller device node to add the i2c adaptor [1] leading
>> to an error spam like below
>>
>> i2c i2c-20: of_i2c: register /soc@...isplay-subsystem@...0000/display-controller@...1000/ports
>> i2c i2c-20: of_i2c: modalias failure on /soc@...isplay-subsystem@...0000/display-controller@...1000/ports
>> i2c i2c-20: Failed to create I2C device for /soc@...isplay-subsystem@...0000/display-controller@...1000/ports
>> i2c i2c-20: of_i2c: register /soc@...isplay-subsystem@...0000/display-controller@...1000/opp-table
>> i2c i2c-20: of_i2c: invalid reg on /soc@...isplay-subsystem@...0000/display-controller@...1000/opp-table
>> i2c i2c-20: Failed to create I2C device for /soc@...isplay-subsystem@...0000/display-controller@...1000/opp-table
>>
>> Add protection against invalid child nodes before trying to register
>> i2c devices for all child nodes.
>>
>> [1] : https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/display/drm_dp_mst_topology.c#L5985
>>
>> Signed-off-by: Abhinav Kumar <quic_abhinavk@...cinc.com>
>> ---
>>   drivers/i2c/i2c-core-of.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/i2c/i2c-core-of.c b/drivers/i2c/i2c-core-of.c
>> index a6c407d36800..62a2603c3092 100644
>> --- a/drivers/i2c/i2c-core-of.c
>> +++ b/drivers/i2c/i2c-core-of.c
>> @@ -86,6 +86,8 @@ void of_i2c_register_devices(struct i2c_adapter *adap)
>>   {
>>          struct device_node *bus, *node;
>>          struct i2c_client *client;
>> +       u32 addr;
>> +       char temp[16];
>>
>>          /* Only register child devices if the adapter has a node pointer set */
>>          if (!adap->dev.of_node)
>> @@ -101,6 +103,10 @@ void of_i2c_register_devices(struct i2c_adapter *adap)
>>                  if (of_node_test_and_set_flag(node, OF_POPULATED))
>>                          continue;
>>
>> +               if (of_property_read_u32(node, "reg", &addr) ||
>> +                   of_alias_from_compatible(node, temp, sizeof(temp)))
>> +                       continue;
> 
> I think just of_property_read_u32() should be enough to skip
> non-I2C-device children. If of_alias_from_compatible() fails, it is a
> legit error.
> 

Thanks for the review.

of_alias_from_compatible() looks for a compatible string but all child 
nodes such as ports will not have the compatible. Hence below error will 
still be seen:

i2c i2c-20: of_i2c: modalias failure on 
/soc@...isplay-subsystem@...0000/display-controller@...1000/ports

>> +
>>                  client = of_i2c_register_device(adap, node);
>>                  if (IS_ERR(client)) {
>>                          dev_err(&adap->dev,
>> --
>> 2.34.1
>>
> 
>