search

dynup / kpatch

kpatch - live kernel patching
GNU General Public License v2.0
1.49k stars 305 forks source link

kmod/patch/livepatch-patch-hook.c: Is the for loop condition 'object->funcs' in patch_free_livepatch neccessary? #1210

Closed he7850 closed 2 years ago

he7850 commented 3 years ago

https://github.com/dynup/kpatch/blob/1ca8e8fc1fa34751a5c7067b82a8319958680a60/kmod/patch/livepatch-patch-hook.c#L230-L247 Is the for loop condition 'object->funcs' in livepatch-patch-hook.c#L235 neccessary?

  1. object->funcs is checked null inside the loop. double checking is redundant.
  2. if object->funcs could be null, then it will break the for loop and may cause memory leak, bcz remaining parts of patch->objs will not be released. I am new to kpatch, if I misunderstand the for loop condition, pls correct me, thx.
jpoimboe commented 3 years ago

I think you're right that the two checks are redundant.

The only way I see object->funcs can be NULL is if patch_init() fails to allocate memory for an instance of lobject->funcs. In that case I don't think there would be a memory leak if patch_free_livepatch() broke out of the loop early because all the subsequent objects in 'patch->objs' would already have NULL funcs and relocs.

So I don't see a memory leak, but you're welcome to submit a pull request to get rid of the redundancy.

he7850 commented 3 years ago

@jpoimboe Thanks for your answer. I have submitted a PR.