netdev - [bug report] net: phylink: add mac_select_pcs() method to phylink_mac

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <ef3eed0e-a4d8-41a5-888a-5333b340c37e@stanley.mountain>
Date: Tue, 22 Oct 2024 11:52:55 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: Russell King <rmk+kernel@...linux.org.uk>
Cc: netdev@...r.kernel.org
Subject: [bug report] net: phylink: add mac_select_pcs() method to
 phylink_mac_ops

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