[PATCH] kdumpctl: Write to `/var/lib/kdump` if $KDUMP_BOOTDIR not writable

[Kelvin Fan]

The /boot directory on some operating systems might be read-only.
If we cannot write to $KDUMP_BOOTDIR when generating the kdump
initrd, attempt to place the generated initrd at `/var/lib/kdump`
instead.

Signed-off by: Kelvin Fan <kfan(a)redhat.com&gt;
---
 kdumpctl | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/kdumpctl b/kdumpctl
index 24f5cf7..2107e3e 100755
--- a/kdumpctl
+++ b/kdumpctl
@@ -152,8 +152,15 @@ rebuild_kdump_initrd()
 rebuild_initrd()
 {
 	if [[ ! -w "$KDUMP_BOOTDIR" ]];then
-		derror "$KDUMP_BOOTDIR does not have write permission. Can not rebuild
$TARGET_INITRD"
-		return 1
+		VAR_TARGET_INITRD_DIR="/var/lib/kdump"
+		mkdir -p "$VAR_TARGET_INITRD_DIR"
+		TARGET_INITRD_BASE="$(basename $TARGET_INITRD)"
+		TARGET_INITRD="$VAR_TARGET_INITRD_DIR/$TARGET_INITRD_BASE"
+		dwarn "$KDUMP_BOOTDIR does not have write permission. Rebuilding initrd at
$TARGET_INITRD"
+		if [[ ! -w "$VAR_TARGET_INITRD_DIR" ]];then
+			derror "$VAR_TARGET_INITRD_DIR does not have write permission. Cannot rebuild
$TARGET_INITRD"
+			return 1
+		fi
 	fi
 
 	if [ $DEFAULT_DUMP_MODE == "fadump" ]; then
@@ -656,6 +663,9 @@ load_kdump()
 		KEXEC_ARGS="$KEXEC_ARGS -s"
 	fi
 
+	# Relabel the target initrd for SELinux.
+	chcon -t boot_t $TARGET_INITRD
+
 	ddebug "$KEXEC $KEXEC_ARGS $standard_kexec_args --command-line=$KDUMP_COMMANDLINE
--initrd=$TARGET_INITRD $KDUMP_KERNEL"
 
 	# The '12' represents an intermediate temporary file descriptor
-- 
2.29.2

[Kairui Song]

Hi Kelvin,

On Tue, Feb 9, 2021 at 9:30 PM Kelvin Fan <kfan(a)redhat.com&gt; wrote:
>  The /boot directory on some operating systems might be read-only.
>  If we cannot write to $KDUMP_BOOTDIR when generating the kdump
>  initrd, attempt to place the generated initrd at `/var/lib/kdump`
>  instead.
>
>  Signed-off by: Kelvin Fan <kfan(a)redhat.com&gt;
>  ---
>   kdumpctl | 14 ++++++++++++--
>   1 file changed, 12 insertions(+), 2 deletions(-)
>
>  diff --git a/kdumpctl b/kdumpctl
>  index 24f5cf7..2107e3e 100755
>  --- a/kdumpctl
>  +++ b/kdumpctl
>  @@ -152,8 +152,15 @@ rebuild_kdump_initrd()
>   rebuild_initrd()
>   {
>          if [[ ! -w "$KDUMP_BOOTDIR" ]];then
>  -               derror "$KDUMP_BOOTDIR does not have write permission. Can not
> rebuild $TARGET_INITRD"
>  -               return 1
>  +               VAR_TARGET_INITRD_DIR="/var/lib/kdump"
>  +               mkdir -p "$VAR_TARGET_INITRD_DIR" 
For this dir, we'd better create it in the spec file. One empty folder
won't hurt anyway.
We still keep a dir detection code here, and create it if doesn't exist.

We may also use this dir for other usage later.

>  +               TARGET_INITRD_BASE="$(basename
> $TARGET_INITRD)"
>  +               TARGET_INITRD="$VAR_TARGET_INITRD_DIR/$TARGET_INITRD_BASE"
>  +               dwarn "$KDUMP_BOOTDIR does not have write permission. Rebuilding
> initrd at $TARGET_INITRD"
>  +               if [[ ! -w "$VAR_TARGET_INITRD_DIR" ]];then
>  +                       derror "$VAR_TARGET_INITRD_DIR does not have write
> permission. Cannot rebuild $TARGET_INITRD"
>  +                       return 1
>  +               fi
>          fi
>
>          if [ $DEFAULT_DUMP_MODE == "fadump" ]; then
>  @@ -656,6 +663,9 @@ load_kdump()
>                  KEXEC_ARGS="$KEXEC_ARGS -s"
>          fi
>
>  +       # Relabel the target initrd for SELinux.
>  +       chcon -t boot_t $TARGET_INITRD
>  + 
Sorry for the delay, after second thought, I think we'd better update
the selinux policy here, instead of changing selinux tag manually
here. This looks a bit hackish.

>          ddebug "$KEXEC $KEXEC_ARGS $standard_kexec_args
> --command-line=$KDUMP_COMMANDLINE --initrd=$TARGET_INITRD $KDUMP_KERNEL"
>
>          # The '12' represents an intermediate temporary file descriptor 
Also can we move these bootdir/initrd path related code to
prepare_kdump_bootinfo() in kdump-lib.sh? That way it won't break
things like early kdump.

And also looking at 60-kdump.install, I think it also needs some
update, so that /var/lib/kdump won't bloat up as kernel updates.

>  --
>  2.29.2
>  _______________________________________________
>  kexec mailing list -- kexec(a)lists.fedoraproject.org
>  To unsubscribe send an email to kexec-leave(a)lists.fedoraproject.org
>  Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
>  List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
>  List Archives:
> https://lists.fedoraproject.org/archives/list/[e-mail] 


--
Best Regards,
Kairui Song

[Kelvin Fan]

Hi Kairui,

Thank you for the review! I have addressed all your comments and
submitted a PATCH v2.

>  Sorry for the delay, after second thought, I think we'd better
> update
>  the selinux policy here, instead of changing selinux tag manually
>  here. This looks a bit hackish. 
Regarding the SELinux bit, where kexec cannot load the kdump initrd if
it's in `/var/lib/kdump`, I've removed the `chcon` line. Do you (or
your team) mind submitting a patch to the Fedora SELinux policy repo
to update the policies regarding this new directory?

This is my first time sending email patches, so I apologize in advance
if I'm not following all the proper conventions.

Thank you!
Kelvin