[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <577c2f8511b700624cdfdf75db5b1a90cf71314b.camel@embedd.com>
Date: Tue, 05 Dec 2023 09:00:39 +0100
From: Daniel Danzberger <dd@...edd.com>
To: Vladimir Oltean <olteanv@...il.com>
Cc: woojung.huh@...rochip.com, UNGLinuxDriver@...rochip.com,
netdev@...r.kernel.org, Andrew Lunn <andrew@...n.ch>, Florian Fainelli
<f.fainelli@...il.com>
Subject: Re: [PATCH] net: dsa: microchip: fix NULL pointer dereference on
platform init
Hi,
On Mon, 2023-12-04 at 19:43 +0200, Vladimir Oltean wrote:
> Hello Daniel,
>
> On Mon, Dec 04, 2023 at 04:43:15PM +0100, Daniel Danzberger wrote:
> > Fixes a NULL pointer access when registering a switch device that has
> > not been defined via DTS.
> >
> > This might happen when the switch is used on a platform like x86 that
> > doesn't use DTS and instantiates devices in platform specific init code.
> >
> > Signed-off-by: Daniel Danzberger <dd@...edd.com>
> > ---
> > drivers/net/dsa/microchip/ksz_common.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
> > index 9545aed905f5..525e13d9e39c 100644
> > --- a/drivers/net/dsa/microchip/ksz_common.c
> > +++ b/drivers/net/dsa/microchip/ksz_common.c
> > @@ -1678,7 +1678,7 @@ static int ksz_check_device_id(struct ksz_device *dev)
> > dt_chip_data = of_device_get_match_data(dev->dev);
> >
> > /* Check for Device Tree and Chip ID */
> > - if (dt_chip_data->chip_id != dev->chip_id) {
> > + if (dt_chip_data && dt_chip_data->chip_id != dev->chip_id) {
> > dev_err(dev->dev,
> > "Device tree specifies chip %s but found %s, please fix it!\n",
> > dt_chip_data->dev_name, dev->info->dev_name);
> > --
> > 2.39.2
> >
> >
>
> Is this all that's necessary for instantiating the ksz driver through
> ds->dev->platform_data? I suppose not, so can you post it all, please?
Yes, that NULL pointer was the only issue I encountered.
Here is the module I use to instantiate the switch, which works fine so far in our
linux v6.1 x86_64 builds:
--
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/i2c.h>
#include <linux/netdevice.h>
#include <net/dsa.h>
static struct i2c_client *i2cdev;
static struct dsa_chip_data ksz9477_dsa = {
.port_names = {
"cpu",
"lan1",
"lan2",
"lan3",
"lan4"
}
};
static struct i2c_board_info t2t_ngr421_i2c_board_info = {
I2C_BOARD_INFO("ksz9477-switch", 0x5f),
.platform_data = &ksz9477_dsa,
};
static int __init t2t_ngr421_platform_init(void)
{
struct i2c_adapter *adapter = i2c_get_adapter(11);
struct net_device *netdev_cpu = NULL, *nd;
if (adapter == NULL) {
pr_err("t2t-ngr421: Missing FT260 I2C Adapter");
return -ENODEV;
}
read_lock(&dev_base_lock);
for_each_netdev(&init_net, nd) {
if (!strcmp(nd->name, "eth0")) {
netdev_cpu = nd;
break;
}
}
read_unlock(&dev_base_lock);
if (netdev_cpu == NULL) {
pr_err("t2t-ngr421: Missing netdev eth0");
return -ENODEV;
}
ksz9477_dsa.netdev[0] = &netdev_cpu->dev;
i2cdev = i2c_new_client_device(adapter, &t2t_ngr421_i2c_board_info);
return i2cdev ? 0 : -ENODEV;
}
static void t2t_ngr421_platform_deinit(void)
{
if (i2cdev)
i2c_unregister_device(i2cdev);
}
module_init(t2t_ngr421_platform_init);
module_exit(t2t_ngr421_platform_deinit);
MODULE_AUTHOR("Daniel Danzberger <dd@...edd.com>");
MODULE_DESCRIPTION("T2T NGR421 platform driver");
MODULE_LICENSE("GPL v2");
--
>
> Looking at dsa_switch_probe() -> dsa_switch_parse(), it expects
> ds->dev->platform_data to contain a struct dsa_chip_data. This is in
> contrast with ksz_spi.c, ksz9477_i2c.c and ksz8863_smi.c, which expect
> the dev->platform_data to have the struct ksz_platform_data type.
> But struct ksz_platform_data does not contain struct dsa_chip_data as
> first element.
Noticed that as well.
But hence the 'struct ksz_platform_data' isn't used anywhere, I passed (see module above) 'struct
dsa_chip_data' directly.
Which is what the DSA code at net/dsa/dsa2.c expects in:
static int dsa_switch_parse(struct dsa_switch *ds, struct dsa_chip_data *cd)
--
Regards
Daniel Danzberger
embeDD GmbH, Alter Postplatz 2, CH-6370 Stans
Powered by blists - more mailing lists