Yeah. You're right. It's just weird that this warning suddenly showed
up in my allmodconfig after so many years. Anyway, let's add the
Cavium people to the CC list and see if they care.
Otherwise, forget about it. Probably people build this as a module if
modules are allowed so it doesn't affect people in real life.
regards,
dan carpenter
On Sun, Oct 08, 2023 at 12:00:00AM +0900, Masahiro Yamada wrote:
> 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
