On Sat, Oct 26, 2024 at 02:55:59PM +0900, Tetsuo Handa wrote:
> If bdev_open() can grab a reference before module's initialization phase
> completes is a problem,
Yes, that would indicate there's a bug and alas we have a regression.
Commit d1909c0221739 ("module: Don't ignore errors from
set_memory_XX()") merged on v6.9 introduced a regression, allowing
module init to start and later us failing module initialization to
complete. So to be clear, there's a possible transition from live to
not live right away.
This was discussed in this thread:
https://lore.kernel.org/all/Zuv0nmFblHUwuT8v@xxxxxxxxxxxxxxxxxxxxxx/T/#u
> I think that we can fix the problem with just
>
> int bdev_open(struct block_device *bdev, blk_mode_t mode, void *holder,
> const struct blk_holder_ops *hops, struct file *bdev_file)
> {
> (...snipped...)
> ret = -ENXIO;
> if (!disk_live(disk))
> goto abort_claiming;
> - if (!try_module_get(disk->fops->owner))
> + if ((disk->fops->owner && module_is_coming(disk->fops->owner)) || !try_module_get(disk->fops->owner))
> goto abort_claiming;
> ret = -EBUSY;
> if (!bdev_may_open(bdev, mode))
> (...snipped...)
> }
>
> change. It would be cleaner if we can do
>
> bool try_module_get(struct module *module)
> {
> bool ret = true;
>
> if (module) {
> /* Note: here, we can fail to get a reference */
> - if (likely(module_is_live(module) &&
> + if (likely(module_is_live(module) && !module_is_coming(module) &&
> atomic_inc_not_zero(&module->refcnt) != 0))
> trace_module_get(module, _RET_IP_);
> else
> ret = false;
> }
> return ret;
> }
>
> but I don't know if this change breaks something.
As I see it, if we fix the above regression I can't see how a module
being live can transition into coming other than the regression above.
Luis
