| Message ID | 20220513102219.30399-5-arun.ramadoss@microchip.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: dsa: microchip: refactor the ksz switch init function | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Series has a cover letter |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/cc_maintainers | success | CCed 11 of 11 maintainers |
| netdev/build_clang | success | Errors and warnings before: 0 this patch: 0 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 55 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
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 >
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. */
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> --- 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(-)