Messages in this thread | | | Date | Wed, 10 May 2017 11:01:06 -0500 | From | "Gustavo A. R. Silva" <> | Subject | Re: [net-mellanox] question about potential null pointer dereference |
| |
Quoting Ido Schimmel <idosch@mellanox.com>:
> On Wed, May 10, 2017 at 10:36:59AM -0500, Gustavo A. R. Silva wrote: >> >> Hello everybody, >> >> While looking into Coverity ID 1350941 I ran into the following piece of >> code at drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c:1483: >> >> 1483static void mlxsw_sp_fdb_notify_mac_process(struct mlxsw_sp *mlxsw_sp, >> 1484 char *sfn_pl, int rec_index, >> 1485 bool adding) >> 1486{ >> 1487 struct mlxsw_sp_port *mlxsw_sp_port; >> 1488 char mac[ETH_ALEN]; >> 1489 u8 local_port; >> 1490 u16 vid, fid; >> 1491 bool do_notification = true; >> 1492 int err; >> 1493 >> 1494 mlxsw_reg_sfn_mac_unpack(sfn_pl, rec_index, mac, &fid, >> &local_port); >> 1495 mlxsw_sp_port = mlxsw_sp->ports[local_port]; >> 1496 if (!mlxsw_sp_port) { >> 1497 dev_err_ratelimited(mlxsw_sp->bus_info->dev, "Incorrect >> local port in FDB notification\n"); >> 1498 goto just_remove; >> 1499 } >> 1500 >> 1501 if (mlxsw_sp_fid_is_vfid(fid)) { >> 1502 struct mlxsw_sp_port *mlxsw_sp_vport; >> 1503 >> 1504 mlxsw_sp_vport = >> mlxsw_sp_port_vport_find_by_fid(mlxsw_sp_port, >> 1505 fid); >> 1506 if (!mlxsw_sp_vport) { >> 1507 netdev_err(mlxsw_sp_port->dev, "Failed to find a >> matching vPort following FDB notification\n"); >> 1508 goto just_remove; >> 1509 } >> 1510 vid = 0; >> 1511 /* Override the physical port with the vPort. */ >> 1512 mlxsw_sp_port = mlxsw_sp_vport; >> 1513 } else { >> 1514 vid = fid; >> 1515 } >> 1516 >> 1517do_fdb_op: >> 1518 err = mlxsw_sp_port_fdb_uc_op(mlxsw_sp, local_port, mac, fid, >> 1519 adding, true); >> 1520 if (err) { >> 1521 if (net_ratelimit()) >> 1522 netdev_err(mlxsw_sp_port->dev, "Failed to set >> FDB entry\n"); >> 1523 return; >> 1524 } >> 1525 >> 1526 if (!do_notification) >> 1527 return; >> 1528 mlxsw_sp_fdb_call_notifiers(mlxsw_sp_port->learning_sync, >> 1529 adding, mac, vid, >> mlxsw_sp_port->dev); >> 1530 return; >> 1531 >> 1532just_remove: >> 1533 adding = false; >> 1534 do_notification = false; >> 1535 goto do_fdb_op; >> 1536} >> >> >> The issue here is that line 1496 implies that mlxsw_sp_port might be NULL. >> If this is the case, the execution path jumps to line 1532 and then to line >> 1517. All this could end up dereferencing a NULL pointer at line 1522. >> >> Is there any chance for mlxsw_sp_port to be NULL at line 1496 and, at the >> same time, a NULL pointer dereference occurs at line 1522? >> >> I'm trying to figure out if this is a false positive or something that >> actually needs to be fixed. > > In theory, yes, it can happen, but it didn't happen yet. I recently > patched that and now it's in Jiri's queue. I guess he'll send it > tomorrow. > > https://github.com/jpirko/linux_mlxsw/commit/4160fb9ad6eeacba7736cfdbf7f52248432c2e89 >
Great, glad to see it is fixed now.
> Thanks for looking into this! >
Sure thing, thanks for clarifying. :) -- Gustavo A. R. Silva
| |