Messages in this thread | | | Date | Mon, 16 May 2022 14:12:52 +0300 | From | Vladimir Oltean <> | Subject | Re: [RFC Patch net-next v2 4/9] net: dsa: microchip: move port memory allocation to ksz_common |
| |
On Fri, May 13, 2022 at 03:52:14PM +0530, Arun Ramadoss wrote: > ksz8795 and ksz9477 init function initializes the memory to dev->ports > and assigns the ds real number of ports. Since both the routines are > same, moved the allocation of port memory to ksz_switch_register after > init. > > Signed-off-by: Arun Ramadoss <arun.ramadoss@microchip.com> > ---
Does this actually work? ksz8_switch_init() and ksz9477_switch_init() still dereference dev->ports. They are called from dev->dev_ops->init() from ksz_switch_register(). You have moved the devm_kzalloc() to _after_ the dev->dev_ops->init() call. So these functions are accessing memory behind a not-yet-allocated pointer.
> drivers/net/dsa/microchip/ksz8795.c | 8 -------- > drivers/net/dsa/microchip/ksz9477.c | 8 -------- > drivers/net/dsa/microchip/ksz_common.c | 9 +++++++++ > 3 files changed, 9 insertions(+), 16 deletions(-) > > diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c > index b6032b65afc2..91f29ff7256c 100644 > --- a/drivers/net/dsa/microchip/ksz8795.c > +++ b/drivers/net/dsa/microchip/ksz8795.c > @@ -1599,11 +1599,6 @@ static int ksz8_switch_init(struct ksz_device *dev) > > dev->reg_mib_cnt = MIB_COUNTER_NUM; > > - dev->ports = devm_kzalloc(dev->dev, > - dev->info->port_cnt * sizeof(struct ksz_port), > - GFP_KERNEL); > - if (!dev->ports) > - return -ENOMEM; > for (i = 0; i < dev->info->port_cnt; i++) { > mutex_init(&dev->ports[i].mib.cnt_mutex); > dev->ports[i].mib.counters = > @@ -1615,9 +1610,6 @@ static int ksz8_switch_init(struct ksz_device *dev) > return -ENOMEM; > } > > - /* set the real number of ports */ > - dev->ds->num_ports = dev->info->port_cnt; > - > /* We rely on software untagging on the CPU port, so that we > * can support both tagged and untagged VLANs > */ > diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c > index c712a0011367..1a0fd36e180e 100644 > --- a/drivers/net/dsa/microchip/ksz9477.c > +++ b/drivers/net/dsa/microchip/ksz9477.c > @@ -1482,11 +1482,6 @@ static int ksz9477_switch_init(struct ksz_device *dev) > dev->reg_mib_cnt = SWITCH_COUNTER_NUM; > dev->mib_cnt = TOTAL_SWITCH_COUNTER_NUM; > > - dev->ports = devm_kzalloc(dev->dev, > - dev->info->port_cnt * sizeof(struct ksz_port), > - GFP_KERNEL); > - if (!dev->ports) > - return -ENOMEM; > for (i = 0; i < dev->info->port_cnt; i++) { > spin_lock_init(&dev->ports[i].mib.stats64_lock); > mutex_init(&dev->ports[i].mib.cnt_mutex); > @@ -1499,9 +1494,6 @@ static int ksz9477_switch_init(struct ksz_device *dev) > return -ENOMEM; > } > > - /* set the real number of ports */ > - dev->ds->num_ports = dev->info->port_cnt; > - > return 0; > } > > diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c > index fd2f1bd3feb5..717734fe437e 100644 > --- a/drivers/net/dsa/microchip/ksz_common.c > +++ b/drivers/net/dsa/microchip/ksz_common.c > @@ -768,6 +768,15 @@ int ksz_switch_register(struct ksz_device *dev, > if (ret) > return ret; > > + dev->ports = devm_kzalloc(dev->dev, > + dev->info->port_cnt * sizeof(struct ksz_port), > + GFP_KERNEL); > + if (!dev->ports) > + return -ENOMEM; > + > + /* set the real number of ports */ > + dev->ds->num_ports = dev->info->port_cnt; > + > /* Host port interface will be self detected, or specifically set in > * device tree. > */ > -- > 2.33.0 >
| |