See also:
drivers/net/ethernet/mscc/ocelot_mrp.c:254 ocelot_mrp_del_ring_role() error: we previously assumed 'ocelot_port' could be null (see line 247)
regards,
dan carpenter
On Thu, Mar 18, 2021 at 03:07:32PM +0300, Dan Carpenter wrote:
> Hello Horatiu Vultur,
>
> This is a semi-automatic email about new static checker warnings.
>
> The patch 7c588c3e96e9: "net: ocelot: Extend MRP" from Mar 16, 2021,
> leads to the following Smatch complaint:
>
> drivers/net/ethernet/mscc/ocelot_mrp.c:180 ocelot_mrp_del()
> error: we previously assumed 'ocelot_port' could be null (see line 173)
>
> drivers/net/ethernet/mscc/ocelot_mrp.c
> 153 int ocelot_mrp_del(struct ocelot *ocelot, int port,
> 154 const struct switchdev_obj_mrp *mrp)
> 155 {
> 156 struct ocelot_port *ocelot_port = ocelot->ports[port];
> 157 int i;
> 158
> 159 if (!ocelot_port)
> 160 return -EOPNOTSUPP;
> 161
> 162 if (ocelot_port->mrp_ring_id != mrp->ring_id)
> 163 return 0;
> 164
> 165 ocelot_mrp_del_vcap(ocelot, port);
> 166 ocelot_mrp_del_vcap(ocelot, port + ocelot->num_phys_ports);
> 167
> 168 ocelot_port->mrp_ring_id = 0;
> 169
> 170 for (i = 0; i < ocelot->num_phys_ports; ++i) {
>
> This loop tries to verify that all the ports have ring_id == 0 otherwise
> we return. It's slightly confusing why this matters.
>
> 171 ocelot_port = ocelot->ports[i];
> 172
> 173 if (!ocelot_port)
> ^^^^^^^^^^^
> Assume the last element of the array is NULL
>
> 174 continue;
> 175
> 176 if (ocelot_port->mrp_ring_id != 0)
> 177 goto out;
>
> This would be more clear if instead of a "goto out;" it just did a
> direct "return 0;"
>
> 178 }
> 179
> 180 ocelot_mrp_del_mac(ocelot, ocelot_port);
>
> We delete the last mac address of all the ring_ids are zero but if the
> last port is zero it will crash.
>
> 181 out:
> 182 return 0;
>
> regards,
> dan carpenter
