Re: [PATCH net] liquidio: check other_oct before dereferencing

["Alexey V. Vissarionov" <gremlin@altlinux.org>]

Good ${greeting_time}!

On 2025-04-30 13:46:54 -0700, Jacob Keller wrote:

 >> get_other_octeon_device() may return NULL; avoid dereferencing
 >> the other_oct pointer in that case.
 >> @@ -796,10 +796,11 @@ static int liquidio_watchdog(void *param)
 >>
 >> #ifdef CONFIG_MODULE_UNLOAD
 >>		vfs_mask1 =
 >>		READ_ONCE(oct->sriov_info.vf_drv_loaded_mask);
 >> -		vfs_mask2 =
 >> READ_ONCE(other_oct->sriov_info.vf_drv_loaded_mask);
 >> -
 >> -		vfs_referencing_pf = hweight64(vfs_mask1);
 >> -		vfs_referencing_pf += hweight64(vfs_mask2);
 >> +		vfs_referencing_pf = hweight64(vfs_mask1);
 >> +		if (other_oct) {
 >> +			vfs_mask2 =
 >> READ_ONCE(other_oct->sriov_info.vf_drv_loaded_mask);
 >> +			vfs_referencing_pf += hweight64(vfs_mask2);
 >> +		}
 > Obviously crashing when other_oct is NULL is bad..

Yes, even if this happens only when attempting to unload the
module.

 > But is it ok to proceed when it is NULL? Is leaving out the
 > counts ok? I guess I don't really understand what other_oct
 > actually represents here.

As I can see, the vf_drv_loaded_mask is a bitmap containing
the flags for all existing virtual functions of all devices.
So, if there's no other device, its' functions are missing
and marked as unavailable in the vf_drv_loaded_mask.


-- 
Alexey V. Vissarionov
gremlin ПРИ altlinux ТЧК org; +vii-cmiii-ccxxix-lxxix-xlii
GPG: 0D92F19E1C0DC36E27F61A29CD17E2B43D879005 @ hkp://keys.gnupg.net