linux-kernel - smatch usage (was Re: [PATCH v2 4/4] net: mdio: Add RTL9300 MDIO driver)

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: <c656bd3f-4ad0-4c2b-8d91-1c81f7e41c52@alliedtelesis.co.nz>
Date: Wed, 18 Dec 2024 08:40:08 +1300
From: Chris Packham <chris.packham@...iedtelesis.co.nz>
To: Simon Horman <horms@...nel.org>
Cc: linux-kernel@...r.kernel.org, Dan Carpenter <dan.carpenter@...aro.org>
Subject: smatch usage (was Re: [PATCH v2 4/4] net: mdio: Add RTL9300 MDIO
 driver)

(culled the cc list)

On 17/12/2024 23:35, Simon Horman wrote:
> + Dan Carpenter
>
> On Tue, Dec 17, 2024 at 10:47:10AM +1300, Chris Packham wrote:
>> On 17/12/2024 05:48, Simon Horman wrote:
>>> On Mon, Dec 16, 2024 at 04:13:46PM +1300, Chris Packham wrote:
>>>> Add a driver for the MDIO controller on the RTL9300 family of Ethernet
>>>> switches with integrated SoC. There are 4 physical SMI interfaces on the
>>>> RTL9300 but access is done using the switch ports so a single MDIO bus
>>>> is presented to the rest of the system.
>>>>
>>>> Signed-off-by: Chris Packham <chris.packham@...iedtelesis.co.nz>
> ...
>
>>>> +		if (smi_addr[0] > MAX_SMI_BUSSES)
>>> Hi Chris,
>>>
>>> Should this condition be
>>>
>>> 		if (smi_addr[0] >= MAX_SMI_BUSSES)
>> Yes. You are correct.
>>>> +			return dev_err_probe(dev, -EINVAL, "illegal smi bus number %d\n",
>>>> +					     smi_addr[0]);
>>>> +
>>>> +		if (smi_addr[1] > MAX_SMI_ADDR)
>>>> +			return dev_err_probe(dev, -EINVAL, "illegal smi addr %d\n", smi_addr[1]);
>>>> +
>>>> +		if (fwnode_device_is_compatible(child, "ethernet-phy-ieee802.3-c45"))
>>>> +			priv->smi_bus_isc45[smi_addr[0]] = true;
>>> Otherwise it seems that smi_bus_isc45 may overflow here.
>>>
>>> Flagged by Smatch.
>> Sounds like something I should start looking at for myself. Have you got a
>> link to share?
> Hi Chris,
>
> Smatch is here: https://github.com/error27/smatch
> And my usage of it is informed by
> https://blogs.oracle.com/linux/post/smatch-static-analysis-tool-overview-by-dan-carpenter

Thanks, I did find the repo.or.cz mirror and 
https://lwn.net/Articles/691882/ after I searched a bit.

>
> FWIIW, I run it usking kchecker on individual source files.
>
> I've also CCed the author, Dan Carpenter, for good measure.

Odd thing is I can't seem to reproduce any error report on my buggy 
code. I've tried `make O=build_smatch CHECK="~/src/smatch/smatch 
-p=kernel" C=1` and `O=build_smatch ~/src/smatch/smatch_scripts/kchecker 
--spammy drivers/net/mdio/mdio-realtek-rtl.c`

> ...
>