Add support for building out-of-tree modules

[pcd1193182]

This PR is a first pass at implementing support for out-of-tree modules. The workflow is slightly different from the standard kpatch workflow for building in-tree modules, in large part because out of tree modules have a variety of ways that they are build and varied file structures that are hard to account for in a tool like this. It requires an external source directory that is set up and ready to build, and it may require that a symlink be created to the Module.symvers file depending on where it is located in the external source directory.

If people have strong feelings about better ways to implement this feature, or ways to make it easier to use, I am happy to tweak the approach take in the PR, this is just the way that seemed easiest to me.

This PR passes make check and make unit, and has successfully built patches for the zfs kernel module, which is built out-of-tree.

[sm00th]

This PR passes make check and make unit, and has successfully built patches for the zfs kernel module, which is built out-of-tree.

Could you please also check on a patch for vmlinux and one of the in-tree modules? make unit only tests create-diff-object. Right now in-tree module patches fail because of missing SYMVERS_FILE.

[pcd1193182]

I did run a simple vmlinux patch through the modified code, but not one for an in-tree module. I didn't realize make unit was so limited, and make integration has been very difficult to get working. I have access to ubuntu-16.04 VMs, but Canonical's repositories have stopped serving a lot of the stuff required to run kpatch against 16.04. I could rewrite all of the integration tests to run against 18.04, but that would take some time. And unfortunately I don't have easy access to centos or rhel VMs. I'll run through the process with an in-tree module, make sure that works, address your comments, and then update the PR. Thanks for taking the time to review this!

[jpoimboe]

Don't worry about running the integration tests, we're still working on making those more accessible (see #922). Though @sm00th 's testing suggestions are still a good idea.

[joe-lawrence]

Hi @pcd1193182 , I just fetched https://github.com/pcd1193182/kpatch as a remote, and I don't see how the issues @sm00th brought up were resolved. Just checking to see if those were still in progress, or if you forgot to (force) push a new revisions? Thanks, -- Joe

[pcd1193182]

@joe-lawrence they're resolved, I just haven't pushed the new rev yet. I didn't get any work done on them over the weekend, working on testing an in-tree module now. I'll update the PR once I get that done, which should be in the next hour or three.

[pcd1193182]

I've updated the PR to address the comments that @sm00th left, and I followed the testing steps suggested and fixed the bugs they revealed.

[sm00th]

Looks ok codewise to me.

[joe-lawrence]

Thanks for updating @pcd1193182 , the changes look reasonable, but I'm probably testing this incorrectly, see the attached test.zip for my out-of-tree module sources. When I try to build a kpatch for it, I get a missing file error:

% ./kpatch-build/kpatch-build \
	-c /boot/config-4.18.0-32.el8.x86_64 \
	-v /lib/debug/lib/modules/4.18.0-32.el8.x86_64/vmlinux \
	-s ~/test \
	-t default \
	-e ~/test/test.ko \
	~/test/test.patch 

Using source directory at /home/cloud-user/test
Testing patch file(s)
/home/cloud-user/test/test.patch
Reading special section data
Building original source
Building patched source
/home/cloud-user/test/test.patch
cp: cannot stat '/home/cloud-user/test//.tmp_test.o': No such file or directory
ERROR: kpatch build failed. Check /home/cloud-user/.kpatch/build.log for more details.

Adding an example kpatch-build invocation to the README.md or kpatch-build manpage might be helpful.

See a few nitpick comments inline.

[pcd1193182]

@joe-lawrence looks like there's a bug in the kpatch-gcc change that generates the relative paths where it doesn't remove the leading slash. I've got a probable fix for it, and I tested it with your test.zip and it worked. I'll test it against out-of-tree module and some in-tree modules and update the PR.

[joe-lawrence]

Hi @pcd1193182,

I'm still having the same issues with my test.zip as I did before:

% git log -1
commit 068d93605351f6616160dbd73b92dd42aa3f90d1 (HEAD, paul/master)
Author: Paul Dagnelie <pcd@delphix.com>
Date:   Wed Nov 7 10:25:23 2018 -0800

    readme tweaks

% ./kpatch-build/kpatch-build -s ~/test/ -t test -e ~/test/test.ko ~/test/test.patch 
...
cp: cannot stat '/home/cloud-user/test/.tmp_test.o': No such file or directory

Maybe I'm not invoking it the same way that you did?

Also, should we provide test.zip in some fashion to help demonstrate this enhancement? Maybe a separate markdown page with the source+makefile serving as a howto for out of tree modules? Just a thought. Thanks for working on this!

[pcd1193182]

@joe-lawrence That's very interesting. I'm using almost exactly that invocation (-t default instead of -t test, because make test fails for me in general when trying to build the module), and I get a successful build.

~/kpatch$ ./kpatch-build/kpatch-build -s ~/test/ -t default -e ~/test/test.ko ~/test/test.patch
Using source directory at /export/home/delphix/test
Testing patch file(s)
Reading special section data
Building original source
Building patched source
Extracting new and modified ELF sections
test.o: changed function: test_thread
Patched objects: ./test.ko
Building patch module: livepatch-test.ko
SUCCESS

Based on the fact that it's cp that's failing, there's only a few places in the kpatch-build script it could be failing. Probably .tmp_test.o is getting listed as a changed object but then cleaned up by the build process? Could you maybe run with -ddd and hand me the log? My guess is that this is due to a difference between our environments; I'm running on an Ubuntu 18.04 VM.

[joe-lawrence]

I did futz a bit with the Makefile target to try and come up with something better than "default" target. Usually when I build the OOT module, I just say "make" and let it pick the first/default.

Find attached a few archives, post ./kpatch-build/kpatch-build --skip-cleanup -ddd -s ~/test/ -t test -e ~/test/test.ko ~/test/test.patch:

• home.kpatch.zip - ~/.kpatch/
• home-test.zip - ~/test/
• kpatch-build.zip - kpatch-build output

[pcd1193182]

@joe-lawrence Got it! I had to clone a centos VM to reproduce the issue, but I've sorted it out. The issue is that relobj was being defined in kpatch-gcc before obj was being redefined in the case of a .tmp_*.o type object file, which is produced by make on some platforms (centos, red hat) but not on others (ubuntu).

[joe-lawrence]

Good catch and thanks for debugging this on CentOS! We just had someone ask about building out-of-tree module livepatches on the mailing list, so this is a feature that others are interested in as well.

I don't have any other code comments for the PR. Usually we squash revisions down into one commit (or series if appropriate), so feel free to squash + force push a final rev.

Thanks, -- Joe