Re: [linux-next:master 2112/3285] drivers/net/phy/phylink.c:588 phylink_validate_mask_caps() warn: variable dereferenced before check 'state' (see line 583)

[Sean Anderson <sean.anderson@seco.com>]

On 10/24/22 10:30, Dan Carpenter wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> head:   76cf65d1377f733af1e2a55233e3353ffa577f54
> commit: f392a1846489720fc2e063d1210633b6cf4ec5a4 [2112/3285] net: phylink: provide phylink_validate_mask_caps() helper
> config: microblaze-randconfig-m031-20221023
> compiler: microblaze-linux-gcc (GCC) 12.1.0
> 
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> New smatch warnings:
> drivers/net/phy/phylink.c:588 phylink_validate_mask_caps() warn: variable dereferenced before check 'state' (see line 583)
> 
> Old smatch warnings:
> drivers/net/phy/phylink.c:1104 phylink_change_inband_advert() error: we previously assumed 'pl->pcs' could be null (see line 1087)
> 
> vim +/state +588 drivers/net/phy/phylink.c
> 
> f392a184648972 Russell King (Oracle  2022-10-17  574) void phylink_validate_mask_caps(unsigned long *supported,
> f392a184648972 Russell King (Oracle  2022-10-17  575) 				struct phylink_link_state *state,
> f392a184648972 Russell King (Oracle  2022-10-17  576) 				unsigned long mac_capabilities)
> 34ae2c09d46a2d Russell King (Oracle  2021-11-15  577) {
> 34ae2c09d46a2d Russell King (Oracle  2021-11-15  578) 	__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
> 3e6eab8f3ef93c Sean Anderson         2022-09-20  579  	unsigned long caps;
> 34ae2c09d46a2d Russell King (Oracle  2021-11-15  580) 
> 34ae2c09d46a2d Russell King (Oracle  2021-11-15  581) 	phylink_set_port_modes(mask);
> 34ae2c09d46a2d Russell King (Oracle  2021-11-15  582) 	phylink_set(mask, Autoneg);
> f392a184648972 Russell King (Oracle  2022-10-17 @583) 	caps = phylink_get_capabilities(state->interface, mac_capabilities,
>                                                                                         ^^^^^^^^^^^^^^^^
> b7e9294885b610 Sean Anderson         2022-09-20  584  					state->rate_matching);
>                                                                                         ^^^^^^^^^^^^^^^^^^^^
> Unchecked dereferences
> 
> 3e6eab8f3ef93c Sean Anderson         2022-09-20  585  	phylink_caps_to_linkmodes(mask, caps);
> 34ae2c09d46a2d Russell King (Oracle  2021-11-15  586) 
> 34ae2c09d46a2d Russell King (Oracle  2021-11-15  587) 	linkmode_and(supported, supported, mask);
> f392a184648972 Russell King (Oracle  2022-10-17 @588) 	if (state)
>                                                             ^^^^^
> Check for NULL
> 
> 34ae2c09d46a2d Russell King (Oracle  2021-11-15  589) 		linkmode_and(state->advertising, state->advertising, mask);
> 34ae2c09d46a2d Russell King (Oracle  2021-11-15  590) }
> 

Hm, looks like I missed this during my conversion, but no existing
codepath depends on this behavior.

Russell, should I send a follow-up fixing this, or do you want to
do that when you submit [1]?

--Sean

[1] http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=net-queue&id=ca5662f375de2a44fabcba644a4368c838e1bb22