On Mon, Sep 06, 2021 at 06:08:54PM +0800, Hou Tao wrote: > >> + if (!try_module_get(THIS_MODULE)) > >> + return ERR_PTR(-ENODEV); > > try_module_get(THIS_MODULE) is an indicator for an unsafe pattern. If > > we don't already have a reference it could never close the race. > > > > Looking at the callers: > > > > - nbd_open like all block device operations must have a reference > > already. > Yes. nbd_open() has already taken a reference in dentry_open(). > > - for nbd_genl_connect I'm not an expert, but given that struct > > nbd_genl_family has a module member I suspect the networkinh > > code already takes a reference. > > That was my original though, but the fact is netlink code doesn't take a module reference > > in genl_family_rcv_msg_doit() and netlink uses genl_lock_all() to serialize between module removal > > and nbd_connect_genl_ops calling, so I think use try_module_get() is OK here. How it this going to work? If there was a race you just shortened it, but it can still happen before you call try_module_get. So I think we need to look into how the netlink calling conventions are supposed to look and understand the issues there first.
