kdump.conf gets parsed over and over again

[prudo1]

Running debug=1 bash -x /usr/bin/kdumpctl restart 2>&1 | grep -c "kdump_get_conf_val " on my test system shows that kdump_get_conf_val is called many times during the start of the kdump.service. In particular (using the default kdump.conf) it is called 63 times when a rebuild of the kdump initrd is required, 53 times without rebuilding the kdump initrd and, 12 times with config force_no_rebuild enabled. Each time kdump_get_conf_val is called the whole kdump.conf is parsed. This can lead to performance regressions as reported in #134.

Fix this by moving over to only parse the config once and store it in a way so it can be accessed later again, like it is done in kdumpctl with the OPT array. The problem with this is that the config is needed in (at least) three different places, kdumpctl, the module-setup.sh and, kdump.sh within the kdump initrd.

My suggestion is to make use of the systemd-creds to pass the config to the kdump-capture.service. This could look something like this:

1. Remove all uses of kdump_get_conf_val in kdumpctl only relying on the OPT array.
2. Write the content of OPT to temporary files, with the option name as file name and the value as content of the file.
3. Install the files to /etc/credstore/kdump in the kdump initrd.
4. Add LoadCredential=kdump:/etc/credstore/kdump to kdump-capture.service
5. In kdump.sh the config can be accesses by checking if the file $CREDENTIALS_DIRECTORY/kdump_<option> exists and then reading its content.

Benefits of using systemd-creds I see (besides reducing the number of times kdump.conf needs to be parsed)

1. kdumpctl can easily change/update the config. E.g. it can add the -F or --num-threads option for makedumpfile. This would allow us to move some functionality from kdump.sh to kdumpctl. Making kdump.sh simpler, easier to predict and, ultimately easier to debug.
2. For UKIs the kdump initrd cannot be rebuild to include the local config but systemd-creds can be passed to them.
3. systemd-creds can be encrypted. Although this requires a TPM2 when the credential is supposed to be available in the initrd. See man systemd-creds for details.

[coiby]

Hi @prudo1 ,

Thanks for proposing a plan! I don't know much about systemd-creds. But for Step 1-5, it seems systemd-creds is not necessary. The plan seems to write parsed configs to /etc/credstore/kdump_ and then read the files directly. If that's the case, could writing parsed configs as a dot file that can sourced by kdump.sh a better way? Are we supposed to systemd-creds like follows ?

mkdir /tmp/creds
echo pass123 > /tmp/creds/db_password
echo token456 > /tmp/creds/api_token

CREDENTIALS_DIRECTORY=/tmp/creds systemd-creds cat db_password

I also tried creating a systemd service having LoadCredential=kdump:/tmp/creds but the service fails with "Failed to set up credentials: Protocol error". It seems LoadCredential doesn't allow specifying a folder.

Another concern is about systemd-creds's performance. Its name suggests it's for dealing credentials/secrects so I imagine majority of its code is not for parsing/retrieving config which mean it may be slow for the case of dealing kdump.conf .

As for the benefit 2 of using systemd-creds, if kdump initrd can't be rebuilt, then it will be a huge challenge. What shall users do if they change the dump target?

Btw, parse_config currently indeed removes spaces in the value. I think this is an unexpected behavior of read -r ,

#!/bin/bash
while read -r _opt _val; do
    echo $_opt $_val
done <<<  "ab 'cd      ef'"

The above script will print ab 'cd ef' instead of ab 'cd ef'.

[prudo1]

Hi @coiby,

you are completely right, we don't have to use systemd-creds to achieve the goal. However, given that for UKIs systemd-creds are the only way to pass non-signed data to the initrd (more below), I believe using them also for non-UKIs makes sense. Simply so we only have to maintain a single mechanism to pass the config to the initrd.

About the performance impact I must admit that I don't know. Although I expect/hope it won't be too bad. Especially when we only read them once in the initrd. Everywhere else I'd use the OPT array.

For your example I'm not entirely sure why it's not working. In my experiment I've added the credentials to /etc/credstore/kdump in the initrd and it worked. Although, checking with systemd-creds list in the initrd they were moved to /run/credentials/kdump-capture.service/kdump_*. Not entirely sure what the systemd-creds dracut module does when running...
In my experiments I accessed the credentials directly via read and cat but using systemd-creds cat works as well.

About UKIs in general. I don't think that not being able to rebuild the initrd will be a huge problem for us. The way I see it, while the initrd contained in the UKI cannot be changed it can be extended using addons. So, I expect the initrd contained in the UKI to only be a minimal one that only knows how to handle those addons. All the important bits will be handled by those addons. All in all I expect it to work similar like today's dracut modules. With the only difference that those modules are also pre-built and signed.
The only problem with this approach is that you cannot include any local config into those addons. But that can be done with the systemd-creds. BTW, for UKIs the creds are not contained in the initrd but are injected into it by the UKI stub that runs between the bootloader and the kernel. BTW2, that's one of the reasons why I don't want to implement UKI support directly in the kernel for kexec_file_load. Because that would require the kernel to know where systemd looks for those creds. Which IMHO is not the job of the kernel.

About the spaces in parse_config. I believe it's a quoting issue. I ran the same test but quoted the variables when echoing them. In that case the spaces/tabs are preserved.

[coiby]

Hi @prudo1

If we can't rebuild the kdump initrd for the case of UKI, I think it can be a big challenge for us. systemd-creds itself doesn't have the ability to make data persistent across kernel reboot. I think it currently depends on TPM to save the data beforehand and retrieve the data later when a new kernel is booted. So here are the possible problems that come with TPM,

1. Quite many machines simply don't support TPM
2. TPM can be slow. I tried storing SSH key in TPM but eventually gave up because the significant delay for each ssh access. It seems currently systemd-creds assumes the data will be encrypted before store them in TPM.
3. TPM has limited storage capacity. If there are many components using TPM, we may hit storage limit.

For the space issue of parse_config, thanks for pointing out the problem with the test script! It turns out the test script doesn't capture the essence of parse_config. If, for example, you add the following option to kdump.conf and make parse_config echo "$config_val"

extra_bins "/usr/local/bin/a    c_test"

you will see the spaces get squashed into one.

[prudo1]

Hi @coiby,

About creds and the UKI. The credentials are not stored on the TPM. On a normal boot for UKI they are stored on the $ESP and then injected into the initrd by the UKI stub (see man systemd-stub for details). Something similar will be needed when we add UKI support for kexec_file_load. The TPM only stores the key and decrypts the credentials, if they are encrypted. But that is not a must. Worst case we can keep them unencrypted. That isn't ideal but at least it's not worse than what we de today, where we store all the information unencrypted in the initrd...
BTW, systemd-creds encrypt has an option --with-key= which lets you specify the key to encrypt the credentials with. It also can be auto-initrd where the key stored in the TPM is used, if present, or a fixed zero length key, if not. So even systems without TPM are covered (although in that case confidentiality isn't provided, but that is to be expected...).
Anyway, for me the systemd-creds still sound like the best way forward. Not perfect but still better compared to the other options. But there is still a lot to learn and there might be some blockers I'm not aware of. Unfortunately we will only know if we try it out.

About the spaces in parse_config. I must admit I'm a little bit confused. Do the spaces get squashed into one or are they preserved? The beginning of your paragraph sounds like they should be preserved. But the end says they got squashed...
Especially as when I ran the same test they were preserved...

[coiby]

Thanks for letting me know $ESP is supposed to store credentials files and pointing me to systemd-stub! So I guess ESP//.cred will be used to pass kdump.conf to the kdump kernel. But some questions still remain, for example,

1. s390x and powerpc currently don't use $ESP. What's the plan for these two architectures?
2. Does it require encrypting credentials? According to systemd-stub, *.cred "is supposed to be used to store auxiliary, encrypted, authenticated credentials for use with LoadCredentialEncrypted in the UEFI System Partition" . It feels kdump.conf doesn't fit into the description.

So maybe it's better to reach a consensus with UKI community so our use case can be acknowledged and officially supported?

For the spaces in parse_config, I meant they were squashed but somehow I can't reproduce it anymore i.e. parse_config now preserve the spaces. I'll leave it aside for now. Sorry I didn't write down the details.