On Sat, Oct 7, 2023 at 5:58 PM Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
>
> Hello Masahiro Yamada,
>
> The patch f71be9d084c9: "net: liquidio: fix mixed module-builtin
> object" from Jun 7, 2023 (linux-next), leads to the following Smatch
> static checker warning:
Why is f71be9d084c9 related?
I can compile lio_main.c as built-in even before that commit.
>
> drivers/net/ethernet/cavium/liquidio/lio_main.c:810 liquidio_watchdog()
> error: NULL dereference inside function 'module_refcount()'.
>
> drivers/net/ethernet/cavium/liquidio/lio_main.c
> 800 disable_all_vf_links(other_oct);
> 801 all_vf_links_are_disabled = true;
> 802
> 803 #ifdef CONFIG_MODULE_UNLOAD
>
> This code is old and buggy, but I think it's possibly the recent changes
> which make this bug visible. Modules can be enabled but it doesn't mean
> that this particular driver was built as a module. I think we want to
> test #if MODULE as well?
I do not know because I do not know what was intended
in that code.
"git show 9ff1a9bad867215e4a7ceeef4e9311d1232902fa"
lists people involved in the buggy code.
Please talk to them.
This seems a fishy driver.
drivers/net/ethernet/cavium/liquidio/lio_main.c
is the only driver code that calls module_refcount()
and checks CONFIG_MODULE_UNLOAD.
> 804 vfs_mask1 = READ_ONCE(oct->sriov_info.vf_drv_loaded_mask);
> 805 vfs_mask2 = READ_ONCE(other_oct->sriov_info.vf_drv_loaded_mask);
> 806
> 807 vfs_referencing_pf = hweight64(vfs_mask1);
> 808 vfs_referencing_pf += hweight64(vfs_mask2);
> 809
> --> 810 refcount = module_refcount(THIS_MODULE);
> ^^^^^^^^^^^
> This will crash because THIS_MODULE is NULL when it's built in to the
> kernel.
>
> 811 if (refcount >= vfs_referencing_pf) {
> 812 while (vfs_referencing_pf) {
> 813 module_put(THIS_MODULE);
> 814 vfs_referencing_pf--;
> 815 }
> 816 }
> 817 #endif
> 818 }
> 819
> 820 return 0;
> 821 }
>
> regards,
> dan carpenter
--
Best Regards
Masahiro Yamada
