kpatch-build: sanity check patched filenames

[joe-lawrence]

Run the input patch(es) through lsdiff and then verify that no obviously
unsupported files are directly modified (e.g. assembly .S files).

Signed-off-by: Joe Lawrence joe.lawrence@redhat.com

[joe-lawrence]

I was revisiting issue #902 to see if we could improve the user experience a bit. Here's an enhancement that could head off some low-hanging fruit if the user tries to patch file paths that are obviously unsupported. Since this operates on the patch file itself, feedback is provided fairly early in the process and not after long kernel builds.

Here's what the output looks like with test patches:

% ./kpatch-build/kpatch-build asm.patch 
Using cache at /home/cloud-user/.kpatch/src
Testing patch file(s)
ERROR: /home/cloud-user/github-kpatch/asm.patch: unsupported patch to assembly: arch/x86_64/kernel/entry_64.S
ERROR: Unsupported changes detected.

% ./kpatch-build/kpatch-build lib.patch
Using cache at /home/cloud-user/.kpatch/src
Testing patch file(s)
ERROR: /home/cloud-user/github-kpatch/lib.patch: unsupported patch to lib/: lib/syscall.c
ERROR: /home/cloud-user/github-kpatch/lib.patch: unsupported patch to lib/: lib/A/B/syscall.c
ERROR: Unsupported changes detected.

I can add additional pathname patterns if desired. I'd be interested in other sanity checks that could be applied through simple patch file inspection as well.

[jpoimboe]

Looks good, though I guess our (horribly out-of-date) README should also be updated to make patchutils a real dependency instead of only being needed for unit tests.

[joe-lawrence]

v2: updated the README.md file

[joe-lawrence]

While we're at it, is there anything else that would make sense to check? Other un-patchable files? Directory depth (ie, one that applies with patch -p1) etc?

[jpoimboe]

Not that I can think of, but you might want to look at all the files that are ignored by kpatch-gcc. For example, it ignores lib/ files. That check can probably be removed now? There might be others as well.

[joe-lawrence]

Not that I can think of, but you might want to look at all the files that are ignored by kpatch-gcc. For example, it ignores lib/ files. That check can probably be removed now? There might be others as well.

Here's the interesting portion of that list:

arch/x86/boot/version.o|\
arch/x86/boot/compressed/eboot.o|\
arch/x86/boot/header.o|\
arch/x86/boot/compressed/efi_stub_64.o|\
arch/x86/boot/compressed/piggy.o|\
kernel/system_certificates.o|\
arch/x86/vdso/*|\
arch/x86/entry/vdso/*|\
drivers/firmware/efi/libstub/*|\
arch/powerpc/kernel/prom_init.o|\
lib/*

Already got lib/* but could probably add the vdso and boot dirs.

As far as removing the checks in kpatch-gcc, I don't think we can do that as this commit only checks direct changes to those source files. A header file change could slip through and still modify an end object file, so I think kpatch-gcc needs to keep its checks intact.

[jpoimboe]

As far as removing the checks in kpatch-gcc, I don't think we can do that as this commit only checks direct changes to those source files. A header file change could slip through and still modify an end object file, so I think kpatch-gcc needs to keep its checks intact.

Agreed

[joe-lawrence]

Merging this for now, we can update paths later if we need to.