Re: [PATCH v2 3/3] nbd: fix race between nbd_alloc_config() and module removal
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.
Reply to: