Skip to content

kpatch-build: building OOT livepatch modules for kernels built from source #1234

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

kpatch-build: building OOT livepatch modules for kernels built from source #1234

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 30 additions & 5 deletions kpatch-build/kpatch-build
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -540,13 +540,14 @@ usage() {
echo " (can be specified multiple times)" >&2
echo " -e, --oot-module Enable patching out-of-tree module," >&2
echo " specify current version of module" >&2
echo " -p, --oot-module-src Set OOT module source directory" >&2
echo " -R, --non-replace Disable replace patch (replace is on by default)" >&2
echo " --skip-cleanup Skip post-build cleanup" >&2
echo " --skip-compiler-check Skip compiler version matching check" >&2
echo " (not recommended)" >&2
}

options="$(getopt -o ha:r:s:c:v:j:t:n:o:de:R -l "help,archversion:,sourcerpm:,sourcedir:,config:,vmlinux:,jobs:,target:,name:,output:,oot-module:,debug,skip-gcc-check,skip-compiler-check,skip-cleanup,non-replace" -- "$@")" || die "getopt failed"
options="$(getopt -o ha:p:r:s:c:v:j:t:n:o:de:R -l "help,archversion:,oot-module-src:,sourcerpm:,sourcedir:,config:,vmlinux:,jobs:,target:,name:,output:,oot-module:,debug,skip-gcc-check,skip-compiler-check,skip-cleanup,non-replace" -- "$@")" || die "getopt failed"

eval set -- "$options"

Expand Down Expand Up @@ -609,6 +610,11 @@ while [[ $# -gt 0 ]]; do
OOT_MODULE="$(readlink -f "$2")"
shift
;;
-p|--oot-module-src)
[[ ! -d "$2" ]] && die "OOT module source dir '$2' not found"
OOT_SOURCE="$(readlink -f "$2")"
shift
;;
-R|--non-replace)
KLP_REPLACE=0
;;
Expand Down Expand Up @@ -662,6 +668,16 @@ if [[ -n "$OOT_MODULE" ]] && [[ -z "$USERSRCDIR" ]]; then
exit 1
fi


if [[ -n "$OOT_MODULE" ]] && [[ -z "$OOT_SOURCE" ]]; then
if [[ -e "$(dirname "$OOT_MODULE")/Makefile" ]]; then
OOT_SOURCE="$(dirname "$OOT_MODULE")"
else
warn "--oot-module requires --oot-module-src"
exit 1
fi
fi

# ensure cachedir and tempdir are setup properly and cleaned
mkdir -p "$TEMPDIR" || die "Couldn't create $TEMPDIR"
rm -rf "${TEMPDIR:?}"/*
Expand All @@ -672,7 +688,9 @@ if [[ -n "$USERSRCDIR" ]]; then
warn "--archversion is incompatible with --sourcedir"
exit 1
fi
SRCDIR="$USERSRCDIR"

[[ -z "$OOT_MODULE" ]] && SRCDIR="$USERSRCDIR" || SRCDIR="$OOT_SOURCE"


if [[ -z "$OOT_MODULE" ]]; then
[[ -z "$VMLINUX" ]] && VMLINUX="$SRCDIR"/vmlinux
Expand Down Expand Up @@ -914,6 +932,7 @@ if [[ "$SKIPCOMPILERCHECK" -eq 0 ]]; then
fi

echo "Testing patch file(s)"

cd "$SRCDIR" || die
verify_patch_files
apply_patches
Expand Down Expand Up @@ -1050,9 +1069,12 @@ ERROR=0

# Prepare OOT module symvers file
if [[ -n "$OOT_MODULE" ]]; then
BUILDDIR="/lib/modules/$ARCHVERSION/build/"
cp -f "$SRCDIR/Module.symvers" "$TEMPDIR/Module.symvers" || die
awk '{ print $1 "\t" $2 "\t" $3 "\t" $4}' "${BUILDDIR}/Module.symvers" >> "$TEMPDIR/Module.symvers"
cp -f "$SRCDIR/Module.symvers" "$TEMPDIR/Module.symvers" || die
# merge host symbols only if we do compile for host kernel
if [[ -e "/lib/modules/$ARCHVERSION/build/" ]]; then
BUILDDIR="/lib/modules/$ARCHVERSION/build/"
awk '{ print $1 "\t" $2 "\t" $3 "\t" $4}' "${BUILDDIR}/Module.symvers" >> "$TEMPDIR/Module.symvers"
fi
fi

for i in $FILES; do
Expand Down Expand Up @@ -1161,8 +1183,11 @@ else
fi

cd "$TEMPDIR/patch" || die

if [[ -z "$OOT_MODULE" ]]; then
KPATCH_BUILD="$SRCDIR"
elif [[ ! -e "/lib/modules/$ARCHVERSION/build" ]]; then
KPATCH_BUILD="$USERSRCDIR"
Copy link
Contributor

@sm00th sm00th Nov 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, thanks for the update.

Unfortunately this change breaks the builds that don't specify $USERSRCDIR.

Copy link
Contributor Author

@omatiusha omatiusha Nov 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But, if I understood correctly, that was the whole point of

It seems to me that instead of changing --source semantics it'll be better to add another option for OOT_MODULE case that will define OOT_MODULE_PATH or something in that vein. We then'll be able to keep using SRCDIR normally

In what cases the build will break?

Copy link
Contributor

@sm00th sm00th Nov 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In what cases the build will break?

Sorry, I should have been more clear. This breaks the normal build (without oot_module) without USERSRCDIR In that case SRCDIR should be used for kpatch-build. I think ideally we would keep SRCDIR untouched by OOT_MODULE code and only use OOT_SOURCE directly when needed. Can't tell if that will be easy to do though, we don't normaly build out-of-tree modules so I am not very familiar with that code.

Copy link
Contributor Author

@omatiusha omatiusha Nov 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, if we do not set SRCDIR to OOT_SOURCE and use it directly, that will add to many if's and that is not good. I tried to mitigate the issue in easier way, please check my last update.

Copy link
Contributor Author

@omatiusha omatiusha Nov 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At what line does OOT_MODULE depend on USERSRCDIR?

Copy link
Contributor

@sm00th sm00th Nov 10, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At what line does OOT_MODULE depend on USERSRCDIR?

Line#666 is the main place but there are also others like Line#692 which is inside if [[ -n "$USERSRCDIR" ]]; then clause.

Copy link
Contributor Author

@omatiusha omatiusha Nov 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, i don't think it shouldn't depend on USERSRCDIR. Where else can we get working Kbuild?

Copy link
Contributor

@sm00th sm00th Nov 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Normally (for non-oot cases) if USERSRCDIR is not specified kpatch-build would download running distro kernel's sources and prepare them for build in SRCDIR (~/.kpatch/src by default). There is this whole section handling it. I think that can work for OOT modules as well.

else
KPATCH_BUILD="/lib/modules/$ARCHVERSION/build"
fi
Expand Down