On Fri, Aug 25, 2017 at 03:10:00PM -0400, Joe Lawrence wrote: > +Test 6 > +------ > + > +Test a scenario where a vmlinux pre-patch callback returns a non-zero > +status (ie, failure): > + > +- load target module > +- load livepatch -ENODEV > +- unload target module > + > +First load a target module: > + > + % insmod samples/livepatch/livepatch-callbacks-mod.ko > + [ 80.740520] livepatch_callbacks_mod: livepatch_callbacks_mod_init > + > +Load the livepatch module, setting its 'pre_patch_ret' value to -19 > +(-ENODEV). When its vmlinux pre-patch callback executed, this status > +code will propagate back to the module-loading subsystem. The result is > +that the insmod command refuses to load the livepatch module: > + > + % insmod samples/livepatch/livepatch-callbacks-demo.ko pre_patch_ret=-19 > + [ 82.747326] livepatch: enabling patch 'livepatch_callbacks_demo' > + [ 82.747743] livepatch: 'livepatch_callbacks_demo': initializing unpatching transition > + [ 82.747767] livepatch_callbacks_demo: pre_patch_callback: vmlinux > + [ 82.748237] livepatch: pre-patch callback failed for object 'vmlinux' > + [ 82.748637] livepatch: failed to enable patch 'livepatch_callbacks_demo' > + [ 82.749059] livepatch: 'livepatch_callbacks_demo': canceling transition, unpatching > + [ 82.749060] livepatch: 'livepatch_callbacks_demo': completing unpatching transition > + [ 82.749177] livepatch_callbacks_demo: post_unpatch_callback: livepatch_callbacks_mod -> [MODULE_STATE_LIVE] Normal state > + [ 82.749868] livepatch: 'livepatch_callbacks_demo': unpatching complete > + [ 82.765809] insmod: ERROR: could not insert module samples/livepatch/livepatch-callbacks-demo.ko: No such device > + > + % rmmod samples/livepatch/livepatch-callbacks-mod.ko > + [ 84.774238] livepatch_callbacks_mod: livepatch_callbacks_mod_exit First off, this documentation is very nice because it clarifies all the callback scenarios and edge cases. The above situation still seems a little odd to me. If I understand correctly, the target module was never patched, and its pre_patch callback was never called. But its post_unpatch callback *was* called. That doesn't seem right. Maybe we should change the condition a little bit. Currently it's: No post-patch, pre-unpatch, or post-unpatch callbacks will be executed for a given klp_object if its pre-patch callback returned non-zero status. I think that might have been my idea, but seeing the above case makes it clear that it's not quite right. Maybe it should instead be: No post-patch, pre-unpatch, or post-unpatch callbacks will be executed for a given klp_object if the object failed to patch, due to a failed pre_patch callback or for any other reason. If the object did successfully patch, but the patch transition never started for some reason (e.g., if another object failed to patch), only the post-unpatch callback will be called. So then, instead of tracking whether the pre-patch callback succeeded, we just need to track whether the object was patched (which we already do, with obj->patched). What do you think? -- Josh -- To unsubscribe from this list: send the line "unsubscribe live-patching" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
