[bug report] net: phylink: add mac_select_pcs() method to phylink_mac_ops

From: Dan Carpenter <hidden>
Date: 2024-10-22 08:53:01

Hello Russell King (Oracle),

Commit d1e86325af37 ("net: phylink: add mac_select_pcs() method to
phylink_mac_ops") from Dec 15, 2021 (linux-next), leads to the
following Smatch static checker warning:

	drivers/net/phy/phylink.c:1208 phylink_major_config()
	error: potential NULL/IS_ERR bug 'pcs'

drivers/net/phy/phylink.c
    1160 static void phylink_major_config(struct phylink *pl, bool restart,
    1161                                   const struct phylink_link_state *state)
    1162 {
    1163         struct phylink_pcs *pcs = NULL;
    1164         bool pcs_changed = false;
    1165         unsigned int rate_kbd;
    1166         unsigned int neg_mode;
    1167         int err;
    1168 
    1169         phylink_dbg(pl, "major config %s\n", phy_modes(state->interface));
    1170 
    1171         pl->pcs_neg_mode = phylink_pcs_neg_mode(pl->cur_link_an_mode,
    1172                                                 state->interface,
    1173                                                 state->advertising);
    1174 
    1175         if (pl->mac_ops->mac_select_pcs) {
    1176                 pcs = pl->mac_ops->mac_select_pcs(pl->config, state->interface);
                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
There are two places where this function pointer is called and they both check
for error pointers.  However these functions never return error pointers, they
return NULL.

    1177                 if (IS_ERR(pcs)) {
    1178                         phylink_err(pl,
    1179                                     "mac_select_pcs unexpectedly failed: %pe\n",
    1180                                     pcs);
    1181                         return;
    1182                 }
    1183 
    1184                 pcs_changed = pl->pcs != pcs;
    1185         }
    1186 
    1187         phylink_pcs_poll_stop(pl);
    1188 
    1189         if (pl->mac_ops->mac_prepare) {
    1190                 err = pl->mac_ops->mac_prepare(pl->config, pl->cur_link_an_mode,
    1191                                                state->interface);
    1192                 if (err < 0) {
    1193                         phylink_err(pl, "mac_prepare failed: %pe\n",
    1194                                     ERR_PTR(err));
    1195                         return;
    1196                 }
    1197         }
    1198 
    1199         /* If we have a new PCS, switch to the new PCS after preparing the MAC
    1200          * for the change.
    1201          */
    1202         if (pcs_changed) {
    1203                 phylink_pcs_disable(pl->pcs);
    1204 
    1205                 if (pl->pcs)
    1206                         pl->pcs->phylink = NULL;
    1207 
--> 1208                 pcs->phylink = pl;
                         ^^^^^^^^^^^^
Potential NULL dereference?

    1209 
    1210                 pl->pcs = pcs;
    1211         }

regards,
dan carpenter
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help