> An LLM found it while auditing kernel/livepatch/. It is not a > production issue. The reproducer was written by an LLM as well; it is > below, apply it with "git apply -p0". It keeps a transition stuck on > a few kthreads and reloads a patched module in a loop. On v7.3-rc7 it > faults in klp_find_ops() within a few minutes with KCSAN_STRICT=y.
Ok, kthreads certainly make it easier. > > I wonder whether we could just decline delete_module() in this case and > > return -EBUSY. It should be a rare operation anyway. Alternatively we > > could increase refcounts of all patched modules during a transition and > > decrease them after. > > I don't think either covers all the paths. klp_module_going() is also > called when a module's init fails, or when its load is aborted after > klp_module_coming(), and neither -EBUSY nor a module reference prevents > that. Loading a patched module also adds to klp_ops under the walk. > And klp_find_ops() walks the global klp_ops list, so it's the modules of > all patches that matter, not just the transition patch. Correct. You would need to check all and you would need to hook up to the module loader differently to the current state. > > I am not saying we should not do the RCU protection but it feels like a > > can of worms a bit so if there is an easier way... > > I could split it: one patch RCU-protecting klp_ops, which fixes the > crash, and a second one for the ordering on module reload, which was found > by inspection and has not been reproduced. Would that be easier to review? Please. It should always be well structured (a logical change per a separate patch and such). Let's explore Petr's proposal first though. Miroslav
