lkrg icon indicating copy to clipboard operation
lkrg copied to clipboard

copy-builtin.sh produces faulty security/lkrg/Makefile

Open berkley4 opened this issue 2 years ago • 6 comments

I'm trying to build the latest 5.10 linux-hardened with the latest git pull of lkrg, using the copy-builtin.sh script. This has worked flawlessly in the past, but since 0cf357d9c08588ed0dd180a52bb953378f9ea948 an incorrect security/lkrg/Makefile is produced.

KBUILD_VERBOSE=1 make distclean

errors out with.....

make -f ./scripts/Makefile.clean obj=security/lkrg
security/lkrg/Makefile:5: *** extraneous 'endif'.  Stop.
make[1]: *** [scripts/Makefile.clean:71: security/lkrg] Error 2
make: *** [Makefile:1840: _clean_security] Error 2

The top of security/lkrg/Makefile is where the trouble appears to lie.....

# SPDX-License-Identifier: GPL-2.0-only

obj-$(CONFIG_SECURITY_LKRG) := p_lkrg.o
p_lkrg-objs += modules/print_log/p_lkrg_debug_log.o
endif

p_lkrg-objs += modules/ksyms/p_resolve_ksym.o \

It should be as below, ie minus the three lines following obj-$(CONFIG_SECURITY_LKRG) := p_lkrg.o

# SPDX-License-Identifier: GPL-2.0-only

obj-$(CONFIG_SECURITY_LKRG) := p_lkrg.o
p_lkrg-objs += modules/ksyms/p_resolve_ksym.o \

Line 33 of copy-builtin.sh contains the following awk command.....

awk '/^\$\(TARGET\)-objs/,/^$/' "$BASEDIR/../Makefile"

This matches the newly added $(TARGET)-objs on line 26 of Makefile, in addition to the originally intended match on line 30, thereby adding the three extra lines to the newly generated Makefile (including the endif which triggers the error).

Since I'm not building a debug kernel, I have patched copy-builtin.sh like so.....

--- a/scripts/copy-builtin.sh
+++ b/scripts/copy-builtin.sh
@@ -30,7 +30,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
 
 obj-\$(CONFIG_SECURITY_LKRG) := p_lkrg.o
-$(awk '/^\$\(TARGET\)-objs/,/^$/' "$BASEDIR/../Makefile"|sed -e 's|src/||; s|$(TARGET)-objs|p_lkrg-objs|')
+$(sed -n -e 's|src/||' -e 's|$(TARGET)|p_lkrg|' -e '/^p_lkrg-objs.*p_resolve_ksym/,/^$/p' "$BASEDIR/../Makefile")
  
 EOC
 )

I've not yet build and tested a kernel with the above patch applied, but this should produce a Makefile identical to to the one produced prior to the aforementioned commit, and ensure that running 'make distclean' no longer fails.

I would imagine the above isn't the optimal or correct way to fix the issue (perhaps failing when building a debug kernel), so I'll leave this task to someone more knowledgeable.

berkley4 avatar May 10 '22 10:05 berkley4

Thanks, @berkley4!

@oshogbo @sempervictus You want to help fix this?

We could also want to introduce some way to prevent/detect similar breakage going forward.

solardiz avatar May 10 '22 12:05 solardiz

If debug builds just need the addition of p_lkrg-objs += modules/print_log/p_lkrg_debug_log.o to the new Makefile, then the following patch should work.

Note that p_resolve_ksym.o acts as a marker and needs to remain listed first in the assignation (in $BASEDIR/../Makefile) in order for sed to match the whole block (otherwise you would miss the p_lkrg-objs += bit).

--- a/scripts/copy-builtin.sh
+++ b/scripts/copy-builtin.sh
@@ -26,12 +26,26 @@
 EOC
 )
 
+MKFL=$(sed -e 's|$(TARGET)|p_lkrg|' -e 's|src/||' "$BASEDIR/../Makefile")
+OBJS=$(echo "$MKFL" | sed -n '/\/p_resolve_ksym\.o/,/\.o$/p')
+
+# Prepend the assignation of debug object files if DEBUG is set
+if [ -n "$DEBUG" ]; then
+OBJS=$(cat <<EOC
+$(echo "$MKFL" | sed -n '/\/p_lkrg_debug_log\.o/,/\.o$/p')
+
+$OBJS
+EOC
+)
+fi
+
 MAKEFILE=$(cat <<EOC
 # SPDX-License-Identifier: GPL-2.0-only
 
 obj-\$(CONFIG_SECURITY_LKRG) := p_lkrg.o
-$(awk '/^\$\(TARGET\)-objs/,/^$/' "$BASEDIR/../Makefile"|sed -e 's|src/||; s|$(TARGET)-objs|p_lkrg-objs|')
- 
+
+$OBJS
+
 EOC
 )

berkley4 avatar May 11 '22 04:05 berkley4

I've been utterly slammed the last few months and haven't even done a proper setup for the built-in test harness :disappointed:. All for having the script updated to address the debug change. Will try to get a few tests in this week, but not likely to happen till Friday at this rate.

sempervictus avatar May 11 '22 12:05 sempervictus

@sempervictus Are you going to send us a PR fixing this issue? Thanks!

solardiz avatar Jul 14 '22 19:07 solardiz

Thanks for pinging @solardiz - added to "todo" list for the weekend.

sempervictus avatar Jul 14 '22 20:07 sempervictus

#203 builds correctly in the default config, could use some testing for SECURITY_LKRG_DEBUG=y as well as runtime checks for both products.

sempervictus avatar Jul 16 '22 02:07 sempervictus

IIRC, this issue is supposed to be fixed by #203. Closing.

solardiz avatar Oct 18 '22 15:10 solardiz