Messages in this thread | | | Subject | Re: [PATCH v2 0/8] klp-convert | From | Joao Moreira <> | Date | Tue, 26 Mar 2019 17:18:47 -0300 |
| |
On 3/18/19 4:18 PM, Joe Lawrence wrote: > On Fri, Mar 01, 2019 at 11:13:05AM -0300, Joao Moreira wrote: >> Livepatches may use symbols which are not contained in its own scope, >> and, because of that, may end up compiled with relocations that will >> only be resolved during module load. Yet, when the referenced symbols are >> not exported, solving this relocation requires information on the object >> that holds the symbol (either vmlinux or modules) and its position inside >> the object, as an object may contain multiple symbols with the same name. >> Providing such information must be done accordingly to what is specified >> in Documentation/livepatch/module-elf-format.txt. >> >> Currently, there is no trivial way to embed the required information as >> requested in the final livepatch elf object. klp-convert solves this >> problem in two different forms: (i) by relying on a symbol map, which is >> built during kernel compilation, to automatically infers the relocation >> targeted symbol, and, when such inference is not possible (ii) by using >> annotations in the elf object to convert the relocation accordingly to >> the specification, enabling it to be handled by the livepatch loader. >> >> Given the above, add support for symbol mapping in the form of >> Symbols.list file; add klp-convert tool; integrate klp-convert tool into >> kbuild; make livepatch modules discernible during kernel compilation >> pipeline; add data-structure and macros to enable users to annotate >> livepatch source code; make modpost stage compatible with livepatches; >> update livepatch-sample and update documentation. >> >> The patch was tested under three use-cases: >> >> use-case 1: There is a relocation in the lp that can be automatically >> resolved by klp-convert (tested by removing the annotations from >> samples/livepatch/livepatch-annotated-sample.c) >> >> use-case 2: There is a relocation in the lp that cannot be automatically >> resolved, as the name of the respective symbol appears in multiple >> objects. The livepatch contains an annotation to enable a correct >> relocation - reproducible with this livepatch sample: >> www.livewire.com.br/suse/klp/livepatch-sample.1.c >> >> use-case 3: There is a relocation in the lp that cannot be automatically >> resolved similarly as 2, but no annotation was provided in the livepatch, >> triggering an error during compilation - reproducible with this livepatch >> sample: www.livewire.com.br/suse/klp/livepatch-sample.2.c >> >> In comparison with v1, this version of the patch-set: >> - was rebased to kernel 4.19 >> - adds a Symbols.list versioning information >> - brings bug fixes and code improvements to klp-convert sources >> >> This is a patch-set repost, given that a typo in a mail address prevented >> the original submission from being posted to lkml. >> >> [ ... snip ... ] > > Hi Joao, > > Apologies for taking so long to get to this patchset, but I finally > spent last week reviewing and testing. My goal was to write a klp > self-test based on the implementation and your sample module. Along the > way I spotted a few minor bugs and other small suggestions. Instead of > dumping a bunch of code or patch content in my replies, I posted my > rebase and modified branch here: > > https://github.com/joe-lawrence/linux/tree/klp-convert-v2-rebase-review > > I added subject line [squash] tags to individual commits that should be > considered fixups for patches in this set. Those commit logs also > contain [joe: description] tags and my sign-offs for that purpose as > well. > > Hopefully this form of feedback will be easy to digest. I'll reply to > the individual patchs here with high-level comments and a pointer to the > corresponding github patch. Let me know if there are any questions. If > it is easier to simply repost as a v3 with those changes, I can do that > as well -- whichever method is easier for you. >
Hi Joe, again thanks a lot for the review. To the things which I had something to say, I already replied to the respective messages. If none has issues with the fix for the ppc64le .TOC. symbols issue (on patch 5/8) and with the fix for the multi-used-m Makefile (on patch 2/8), I guess we are good for moving forward to a v3.
If you don't mind, this is fine by me that you squash the changes and post the newer version. In fact, I can't figure out why my patch submissions did not appear in lkml (if someone knows what could be the reason, please, let me know), so I guess it would be nice to have it reachable this time.
Tks, Joao
> I'll also take comments on my work-in-progress self-test for > klp-convert: > > [new] livepatch/selftests: add klp-convert > https://github.com/torvalds/linux/commit/b0d858b9356d3c909096509a0f18e092b739b44f > > At the moment, it consists of two livepatch modules (I'd prefer to > consolidate, but ran into an issue with klp-convert and multiple object > files) and verifies that livepatch can correctly resolve sympos of 0/1, > 2 for unique/non-uniquely named strings and functions. > > -- Joe >
| |