On Tue, Aug 07, 2018 at 06:13:29PM +0300, Evgenii Shatokhin wrote: > Hi, > > If there are any ideas how to safely apply fixes like > http://patchwork.ozlabs.org/patch/952304/ ("netfilter: ipset: fix > ip_set_list allocation failure") via a live patch, it would be great. > > Essentially, the patch replaces the calls to kmalloc and kfree with kvmalloc > and kvfree, respectively: > ---------------------- > @@ -2059,7 +2059,7 @@ ip_set_net_init(struct net *net) > if (inst->ip_set_max >= IPSET_INVALID_ID) > inst->ip_set_max = IPSET_INVALID_ID - 1; > > - list = kcalloc(inst->ip_set_max, sizeof(struct ip_set *), GFP_KERNEL); > + list = kvcalloc(inst->ip_set_max, sizeof(struct ip_set *), GFP_KERNEL); > if (!list) > return -ENOMEM; > inst->is_deleted = false; > @@ -2087,7 +2087,7 @@ ip_set_net_exit(struct net *net) > } > } > nfnl_unlock(NFNL_SUBSYS_IPSET); > - kfree(rcu_dereference_protected(inst->ip_set_list, 1)); > + kvfree(rcu_dereference_protected(inst->ip_set_list, 1)); > } > > static struct pernet_operations ip_set_net_ops = { > ---------------------- > > However, this patch is unsafe to apply as is in runtime. Consider the > following scenario: > > 1. The live patch is loaded. > 2. The user creates a new net ns or whatever, so that the *patched* > ip_set_net_init() is called and calls kvcalloc(). Let us assume kvcalloc() > uses vmalloc() this time. > 3. The live patch is unloaded for some reason. > 4. The user destroys the net ns and the *original* ip_set_net_exit() is > called. The address of the memory area allocated with vmalloc at step 2 will > be passed to kfree() here. The kernel will crash. > > Currently, I cannot see how to provide such fixes in a live patch without > additional hacks. > > As for the hacks, I could probably add a pre-patch callback that finds the > call to kfree() in the original ip_set_net_exit() and permanently replaces > it with a call to kvfree(). This way, even after the live patch has been > unloaded at step 3, the memory will be freed correctly at the step 4. Not > very elegant, but might work. > > As I am only interested in doing this on x86_64 right now, finding the > needed call in the binary code of ip_set_net_exit() is not a problem. As for > replacement itself - well, I guess something like text_poke* or a part of > its implementation could be used here. > > Still, I wonder if there is a better way to apply such fixes in runtime, > ideally, without additional hacks. > > Any ideas?
I'm thinking the only non-hackish way to prevent the above scenario is to prevent the module from being unloaded. Maybe with a try_module_get(THIS_MODULE) from the pre-patch hook? However, I think there are also other races: 1) Before the patch, a netns is created, and the unpatched ip_set_net_init() is called, which uses kcalloc(). After the patch, the netns is destroyed, and the patched ip_set_net_exit() is called, which uses kvfree(). Boom. 2) During the patch, patched thread A calls ip_set_net_init(), then unpatched thread B calls ip_set_net_exit(). Boom. This is unlikely, but possible if net namespaces are being created/destroyed during the patching process. One solution for #1 involves using a shadow variable. In the init function, create a dummy shadow variable in ip_set_net_init() which indicates that kvcalloc() was used. Then in the exit function, if the shadow variable exists, use kvfree(), else use kfree(). The solution for #2 is a little trickier. One option might be to grab the appropriate lock (net->nsid_lock?) in the pre-patch callback and release it in the post-patch callback. Another option might be to split it into two patches, with the exit patch loaded first and the init patch loaded second. We may want to figure out a way to support this scenario a little better in upstream livepatch. For example, maybe we could have an interface to split a patch into subpatches which are applied in a certain order. More ideas welcome... BTW, if you get it all figured out, an update to the patch-author-guide.md would be appreciated :-) -- Josh _______________________________________________ kpatch mailing list [email protected] https://www.redhat.com/mailman/listinfo/kpatch
