Re: [bug report] net: ocelot: Extend MRP

[Dan Carpenter <dan.carpenter@oracle.com>]

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