Skip to content
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

Jump to bottom

rpm-ostree segfaults with kernel arguments with quotes in them while rebasing to a new version of a container image #3228

Closed
lpuv opened this issue Apr 12, 2024 · 7 comments · Fixed by coreos/rpm-ostree#4915
Closed

rpm-ostree segfaults with kernel arguments with quotes in them while rebasing to a new version of a container image #3228

lpuv opened this issue Apr 12, 2024 · 7 comments · Fixed by coreos/rpm-ostree#4915
Labels
bug difficulty/low Not extremely difficult reward/medium Fixing this will be notably useful triaged This issue has been evaluated and is valid

Comments

@lpuv
Copy link

lpuv commented Apr 12, 2024 •
edited
Loading

Relevant logs from systemd:

Apr 04 17:41:10 cattop rpm-ostree[30502]: ostree chunk layers already present: 65
Apr 04 17:41:10 cattop rpm-ostree[30502]: custom layers already present: 1
Apr 04 17:41:10 cattop rpm-ostree[30502]: custom layers needed: 1 (2.1 GB)
Apr 04 17:41:10 cattop rpm-ostree[30502]: layers already present: 66; layers needed: 1 (2.1 GB)
Apr 04 17:41:10 cattop rpm-ostree[30502]: Fetching layer sha256:08334e94601c (2.1 GB)
Apr 04 17:42:43 cattop rpm-ostree[30502]: Fetched layer sha256:08334e94601c
Apr 04 17:43:10 cattop rpm-ostree[30502]: **
Apr 04 17:43:10 cattop rpm-ostree[30502]: OSTree:ERROR:src/libostree/ostree-kernel-args.c:197:split_kernel_args: 'quoted' should be FALSE
Apr 04 17:43:10 cattop rpm-ostree[30502]: Bail out! OSTree:ERROR:src/libostree/ostree-kernel-args.c:197:split_kernel_args: 'quoted' should be FALSE
Apr 04 17:43:11 cattop systemd[1]: rpm-ostreed.service: Main process exited, code=dumped, status=6/ABRT
Apr 04 17:43:11 cattop systemd[1]: rpm-ostreed.service: Failed with result 'core-dump'.

The kernel argument causing issues is: acpi_osi="!Windows 2020"

This issue can be mitigated by removing the kernel argument using the editor, doing the rebase, and then adding it with --append
Interestingly, when using append it will strip the quotes.

As requested, linking PR related: #3208
@cgwalters cgwalters added bug triaged This issue has been evaluated and is valid reward/medium Fixing this will be notably useful difficulty/low Not extremely difficult labels Apr 12, 2024
@travier
Copy link
Member

travier commented Apr 12, 2024

@lpuv Can you add information about which version of ostree/rpm-ostree you're using? Thanks

$ rpm-ostree status
$ rpm -qa | grep ostree

@ericcurtin
Copy link
Collaborator

ericcurtin commented Apr 12, 2024 •
edited
Loading

Relevant logs from systemd:

Apr 04 17:41:10 cattop rpm-ostree[30502]: ostree chunk layers already present: 65
Apr 04 17:41:10 cattop rpm-ostree[30502]: custom layers already present: 1
Apr 04 17:41:10 cattop rpm-ostree[30502]: custom layers needed: 1 (2.1 GB)
Apr 04 17:41:10 cattop rpm-ostree[30502]: layers already present: 66; layers needed: 1 (2.1 GB)
Apr 04 17:41:10 cattop rpm-ostree[30502]: Fetching layer sha256:08334e94601c (2.1 GB)
Apr 04 17:42:43 cattop rpm-ostree[30502]: Fetched layer sha256:08334e94601c
Apr 04 17:43:10 cattop rpm-ostree[30502]: **
Apr 04 17:43:10 cattop rpm-ostree[30502]: OSTree:ERROR:src/libostree/ostree-kernel-args.c:197:split_kernel_args: 'quoted' should be FALSE
Apr 04 17:43:10 cattop rpm-ostree[30502]: Bail out! OSTree:ERROR:src/libostree/ostree-kernel-args.c:197:split_kernel_args: 'quoted' should be FALSE
Apr 04 17:43:11 cattop systemd[1]: rpm-ostreed.service: Main process exited, code=dumped, status=6/ABRT
Apr 04 17:43:11 cattop systemd[1]: rpm-ostreed.service: Failed with result 'core-dump'.

The kernel argument causing issues is: acpi_osi="!Windows 2020"

This issue can be mitigated by removing the kernel argument using the editor, doing the rebase, and then adding it with --append Interestingly, when using append it will strip the quotes.

If on a BLS based bootloader (most people are, modern grub is this), one way you can do this is to just edit the BLS file and reboot to change cmdline (just be careful as usual when changing boot configuration to only edit the latest file so you have something to rollback to):

/boot/loader/entries/

I really wish this API was as simple as this:

  1. A command to print the full configured cmdline
  2. A command to set the full configured cmdline

and no modifying individual kargs, because that parsing is hairy.

If we revisit this PR and editing individual kargs @HuijingHei there are other karg parsers around like the recently retired https://github.com/rhboot/grubby :

#3208

As requested, linking PR related: #3208

@HuijingHei
Copy link
Collaborator

HuijingHei commented Apr 12, 2024 •
edited
Loading

Thanks @lpuv @ericcurtin for the report!

Can reproduce this with ostree-2024.5-1.fc40.x86_64, will look at it.

[root@cosa-devsh ~]# rpm -q ostree
ostree-2024.5-1.fc40.x86_64

[root@cosa-devsh ~]# rpm-ostree kargs
... acpi_osi="!Windows 2020"

[root@cosa-devsh ~]# cat /boot/loader/entries/ostree-2.conf 
... acpi_osi="!Windows 2020"


[root@cosa-devsh ~]# rpm-ostree rebase ostree-unverified-registry:quay.io/fedora/fedora-coreos:next
Pulling manifest: ostree-unverified-registry:quay.io/fedora/fedora-coreos:next
Importing: ostree-unverified-registry:quay.io/fedora/fedora-coreos:next (digest: sha256:777b335529488aaaddc80730008c56fe68e376479e9e76a8b52ceb0cc932e7bb)
ostree chunk layers needed: 51 (735.4 MB)
Staging deployment... done
error: Bus owner changed, aborting. This likely means the daemon crashed; check logs with `journalctl -xe`.

[root@cosa-devsh ~]# journalctl | grep rpm-ostree
...
Apr 12 11:25:32 cosa-devsh rpm-ostree[1308]: Fetching ostree chunk sha256:d82f76724569 (1.5 MB)
Apr 12 11:25:33 cosa-devsh rpm-ostree[1308]: Fetched ostree chunk sha256:d82f76724569
Apr 12 11:25:38 cosa-devsh rpm-ostree[1308]: **
Apr 12 11:25:38 cosa-devsh rpm-ostree[1308]: OSTree:ERROR:src/libostree/ostree-kernel-args.c:197:split_kernel_args: 'quoted' should be FALSE
Apr 12 11:25:38 cosa-devsh rpm-ostree[1308]: Bail out! OSTree:ERROR:src/libostree/ostree-kernel-args.c:197:split_kernel_args: 'quoted' should be FALSE
Apr 12 11:25:38 cosa-devsh audit[1308]: ANOM_ABEND auid=4294967295 uid=0 gid=0 ses=4294967295 subj=system_u:system_r:install_t:s0 pid=1308 comm="pool-rpm-ostree" exe="/usr/>
Apr 12 11:25:38 cosa-devsh systemd[1]: Created slice system-systemd\x2dcoredump.slice - Slice /system/systemd-coredump.

@dbnicholson
Copy link
Member

I was going to suggest this the first time, but I wonder if g_shell_parse_argv can be used. It's certainly had any bugs shaken out by now and does the quote stripping that's needed.

$ python3
Python 3.11.2 (main, Mar 13 2023, 12:18:29) [GCC 12.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from gi.repository import GLib
>>> with open('/proc/cmdline') as f:
...     kcmdline = f.read()
... 
>>> kcmdline = 'foo="a b c" ' + kcmdline
>>> kcmdline
'foo="a b c" BOOT_IMAGE=/boot/vmlinuz-6.5.0-10-generic root=UUID=4cd38130-0727-4d63-95c2-d11c60a07d8b ro rw splash plymouth.ignore-serial-consoles quiet loglevel=0\n'
>>> GLib.shell_parse_argv(kcmdline)
(True, argvp=['foo=a b c', 'BOOT_IMAGE=/boot/vmlinuz-6.5.0-10-generic', 'root=UUID=4cd38130-0727-4d63-95c2-d11c60a07d8b', 'ro', 'rw', 'splash', 'plymouth.ignore-serial-consoles', 'quiet', 'loglevel=0'])

@lpuv
Copy link
Author

lpuv commented Apr 12, 2024

@lpuv Can you add information about which version of ostree/rpm-ostree you're using? Thanks

$ rpm-ostree status
$ rpm -qa | grep ostree

State: idle
AutomaticUpdates: stage; rpm-ostreed-automatic.timer: no runs since boot
Deployments:
  ostree-image-signed:docker://ghcr.io/lpuv/leo-fedora:39
                   Digest: sha256:550576326a352cbc5d8773476f1c3c10b65283bec4d30feed2008b6a4c56e0d1
                  Version: 39.20240411.0 (2024-04-11T17:18:53Z)
                     Diff: 64 upgraded, 3 added

● ostree-image-signed:docker://ghcr.io/lpuv/leo-fedora:latest
                   Digest: sha256:d7d75a050e9d0b7d9a33f79bbe460e1289c17d00c4edb28ca074be8b43b887d6
                  Version: 39.20240403.0 (2024-04-03T16:50:56Z)

  ostree-image-signed:docker://ghcr.io/lpuv/leo-fedora:latest
                   Digest: sha256:a8a8e3758143508cee79ae003ae84b9d417130d9a9d08cc034cb82892910ad7c
                  Version: 39.20240323.0 (2024-03-24T02:21:52Z)

fedora-release-ostree-desktop-39-36.noarch
ostree-libs-2024.5-1.fc39.x86_64
rpm-ostree-libs-2024.4-3.fc39.x86_64
ostree-2024.5-1.fc39.x86_64
rpm-ostree-2024.4-3.fc39.x86_64
ostree-grub2-2024.5-1.fc39.x86_64
fedora-repos-ostree-39-1.noarch

@lpuv
Copy link
Author

lpuv commented Apr 12, 2024

Additionally, when using my mitigation method the kernel boots with "acpi_osi=!Windows 2020" instead of acpi_osi="!Windows 2020"

HuijingHei added a commit to HuijingHei/rpm-ostree that referenced this issue Apr 15, 2024
@HuijingHei
Keep spaces in double quotes
0864e32 
Related change in ostree is ostreedev/ostree#3208.

Fixes: ostreedev/ostree#3228
HuijingHei added a commit to HuijingHei/rpm-ostree that referenced this issue Apr 15, 2024
@HuijingHei
Keep spaces in double quotes
e28f2e2 
Related change in ostree is ostreedev/ostree#3208.

Fixes: ostreedev/ostree#3228
@HuijingHei HuijingHei mentioned this issue Apr 15, 2024
Merged
@HuijingHei
Copy link
Collaborator

I was going to suggest this the first time, but I wonder if g_shell_parse_argv can be used. It's certainly had any bugs shaken out by now and does the quote stripping that's needed.

$ python3
Python 3.11.2 (main, Mar 13 2023, 12:18:29) [GCC 12.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from gi.repository import GLib
>>> with open('/proc/cmdline') as f:
...     kcmdline = f.read()
... 
>>> kcmdline = 'foo="a b c" ' + kcmdline
>>> kcmdline
'foo="a b c" BOOT_IMAGE=/boot/vmlinuz-6.5.0-10-generic root=UUID=4cd38130-0727-4d63-95c2-d11c60a07d8b ro rw splash plymouth.ignore-serial-consoles quiet loglevel=0\n'
>>> GLib.shell_parse_argv(kcmdline)
(True, argvp=['foo=a b c', 'BOOT_IMAGE=/boot/vmlinuz-6.5.0-10-generic', 'root=UUID=4cd38130-0727-4d63-95c2-d11c60a07d8b', 'ro', 'rw', 'splash', 'plymouth.ignore-serial-consoles', 'quiet', 'loglevel=0'])

It looks good and convenient when using python, also write to file as expected (with quotes).

# cat test.py
from gi.repository import GLib

with open('/proc/cmdline') as f:
    kcmdline = f.read()

kcmdline = 'foo="a b c" ' + kcmdline
output_file = "test.txt"
with open(output_file, "w") as file:
    file.write(kcmdline)

# python3 test.py && cat test.txt
foo="a b c" BOOT_IMAGE=(hd0,gpt1)/vmlinuz-6.7.7-100.fc38.x86_64 root=UUID=c4a60d07-fc11-45b8-9bf5-4d6659493bd5 ro rhgb quiet

When using c, seems it strips the quotes, we might need condition to add " when there is space in value, maybe can keep as it is and change to g_shell_parse_argv if there is any improvement on it. @dbnicholson WDYT?

# cat split.c
#include <stdio.h>
#include <string.h>
#include <glib.h>
int main(int argc, char **argv) {
    char **tokens;
    int num_tokens;
    FILE *file;
    char *command = "foo=\"a b c\" bar=test";
    gboolean success = g_shell_parse_argv(command, &num_tokens, &tokens, NULL);

    if (success) {
        file = fopen("test.txt", "w");
        for (int i = 0; i < num_tokens; i++) {
            g_debug("%s\n", tokens[i]);
            fprintf(file, "%s ", tokens[i]);
        }
        fclose(file);
        g_strfreev(tokens);
    }

    return 0;
}

[root@e83278bcc56b c]# export G_MESSAGES_DEBUG=all; gcc split.c -o split $(pkg-config --cflags --libs glib-2.0)
[root@e83278bcc56b c]# ./split 
** (process:145): DEBUG: 02:14:54.408: foo=a b c

** (process:145): DEBUG: 02:14:54.410: bar=test

[root@e83278bcc56b c]# cat test.txt 
foo=a b c bar=test

HuijingHei added a commit to HuijingHei/rpm-ostree that referenced this issue Apr 15, 2024
@HuijingHei
kargs: keep spaces in double quotes
fd44781 
Related change in ostree is ostreedev/ostree#3208.

Fixes: ostreedev/ostree#3228
lukewarmtemp pushed a commit to lukewarmtemp/rpm-ostree that referenced this issue Apr 18, 2024
@HuijingHei @lukewarmtemp
kargs: keep spaces in double quotes
5496f3e 
Related change in ostree is ostreedev/ostree#3208.

Fixes: ostreedev/ostree#3228
lukewarmtemp pushed a commit to lukewarmtemp/rpm-ostree that referenced this issue Apr 18, 2024
@HuijingHei @lukewarmtemp
kargs: keep spaces in double quotes
629ecb9 
Related change in ostree is ostreedev/ostree#3208.

Fixes: ostreedev/ostree#3228
lukewarmtemp pushed a commit to lukewarmtemp/rpm-ostree that referenced this issue Apr 18, 2024
@HuijingHei @lukewarmtemp
kargs: keep spaces in double quotes
c5db068 
Related change in ostree is ostreedev/ostree#3208.

Fixes: ostreedev/ostree#3228
lukewarmtemp pushed a commit to lukewarmtemp/rpm-ostree that referenced this issue Apr 22, 2024
@HuijingHei @lukewarmtemp
kargs: keep spaces in double quotes
b2cffd8 
Related change in ostree is ostreedev/ostree#3208.

Fixes: ostreedev/ostree#3228
lukewarmtemp pushed a commit to lukewarmtemp/rpm-ostree that referenced this issue Apr 23, 2024
@HuijingHei @lukewarmtemp
kargs: keep spaces in double quotes
b6052ee 
Related change in ostree is ostreedev/ostree#3208.

Fixes: ostreedev/ostree#3228
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug difficulty/low Not extremely difficult reward/medium Fixing this will be notably useful triaged This issue has been evaluated and is valid
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

kargs: keep spaces in double quotes HuijingHei/rpm-ostree
6 participants
@cgwalters @ericcurtin @dbnicholson @HuijingHei @lpuv @travier