systemd-cgtop の SEGV を直そう

systemd-cgtop という systemd 環境で CGroup ごとの CPU, Memory, Block IO usage を top のように見ることができるコマンドがあります. こんな感じ:

Control Group                                                     Tasks   %CPU   Memory  Input/s Output/s
/                                                                     -  123.4     6.3G        -        -
/system.slice                                                       102   82.9     1.9G        -        -
/system.slice/run-ree3377384dda4871ad4e305532d702d8.scope            50   79.7     1.5G        -        -
/user.slice                                                         578   40.2     3.7G        -        -
/system.slice/sddm.service                                            3    2.5   286.5M        -        -
/system.slice/NetworkManager.service                                  7    0.5     9.3M        -        -
/system.slice/dbus.service                                            1    0.1    26.8M        -        -
/system.slice/avahi-daemon.service                                    2    0.0     1.1M        -        -
/init.scope                                                           1      -     4.2M        -        -
/system.slice/acpid.service                                           1      -   264.0K        -        -
/system.slice/boot-efi.mount                                          -      -    12.0K        -        -
/system.slice/chronyd.service                                         1      -   616.0K        -        -
/system.slice/cronie.service                                          1      -     3.6M        -        -
/system.slice/cups.service                                            1      -     7.8M        -        -
/system.slice/dev-hugepages.mount                                     -      -    16.0K        -        -
/system.slice/dev-mqueue.mount                                        -      -     1.3M        -        -
/system.slice/dev-sda6.swap                                           -      -    20.0K        -        -
/system.slice/distccd.service                                         8      -     1.2M        -        -
/system.slice/geoclue.service                                         4      -     7.8M        -        -
/system.slice/nullmailer.service                                      1      -     1.9M        -        -
/system.slice/polkit.service                                          5      -     4.6M        -        -
/system.slice/rtkit-daemon.service                                    3      -   420.0K        -        -
/system.slice/sshd.service                                            1      -     1.3M        -        -
/system.slice/sys-kernel-config.mount                                 -      -    16.0K        -        -
/system.slice/system-getty.slice                                      1      -   208.0K        -        -
/system.slice/system-getty.slice/getty@tty2.service                   1      -        -        -        -
/system.slice/system-systemd\x2dbacklight.slice                       -      -    16.0K        -        -
/system.slice/system-systemd\x2dcoredump.slice                        -      -   416.0K        -        -
/system.slice/system-systemd\x2dfsck.slice                            -      -    48.0K        -        -
/system.slice/systemd-journald.service                                1      -    12.8M        -        -
/system.slice/systemd-logind.service                                  1      -     1.0M        -        -
/system.slice/systemd-udevd.service                                   1      -     6.0M        -        -
/system.slice/udisks2.service                                         5      -     2.6M        -        -
/system.slice/upower.service                                          3      -     2.5M        -        -
/system.slice/wpa_supplicant.service                                  1      -     1.6M        -        -
/user.slice/user-1000.slice                                         574      -        -        -        -
/user.slice/user-1000.slice/session-1.scope                         572      -        -        -        -
/user.slice/user-1000.slice/user@1000.service                         2      -        -        -        -
/user.slice/user-119.slice                                            4      -        -        -        -
/user.slice/user-119.slice/session-c1.scope                           2      -        -        -        -
/user.slice/user-119.slice/user@119.service                           2      -        -        -        -

ところが, これがうちの環境で SEGV していたので追っかけた話です.

naota ~ # gdb systemd-cgtop
(gdb) r
Starting program: /usr/bin/systemd-cgtop
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".

Program received signal SIGSEGV, Segmentation fault.
refresh_one (controller=<optimized out>, path=0x5555557ac240 "", a=0x5555557ac270, b=0x5555557ac2a0, iteration=0, depth=0, ret=0x0) at /var/tmp/portage/sys-apps/systemd-229/work/systemd-229/src/cgtop/cgtop.c:407
407                         child &&
(gdb) bt
#0  refresh_one (controller=<optimized out>, path=0x5555557ac240 "", a=0x5555557ac270, b=0x5555557ac2a0, iteration=0, depth=0, ret=0x0)
    at /var/tmp/portage/sys-apps/systemd-229/work/systemd-229/src/cgtop/cgtop.c:407
#1  0x000055555555a4d3 in refresh (iteration=<optimized out>, b=0x5555557ac2a0, a=0x5555557ac270, root=0x5555557ac240 "") at /var/tmp/portage/sys-apps/systemd-229/work/systemd-229/src/cgtop/cgtop.c:442
#2  main (argc=<optimized out>, argv=<optimized out>) at /var/tmp/portage/sys-apps/systemd-229/work/systemd-229/src/cgtop/cgtop.c:936
(gdb) p child
$1 = (Group *) 0x7fff00000020
(gdb) list
402                     if (r < 0)
403                             return r;
404
405                     if (arg_recursive &&
406                         IN_SET(arg_count, COUNT_ALL_PROCESSES, COUNT_USERSPACE_PROCESSES) &&
407                         child &&
408                         child->n_tasks_valid &&
409                         streq(controller, SYSTEMD_CGROUP_CONTROLLER)) {
410
411                             /* Recursively sum up processes */

gdb で動かしてみるとやっぱり死ぬので backtrace とソース見たところ. child あたりがあやしいので見ていく

(gdb) p child
$2 = (Group *) 0x7fff00000020
(gdb) p child->n_tasks_valid
Cannot access memory at address 0x7fff00000020

ということで, child に変な値入ってるっぽい. これどうせ初期化忘れだろうなという気持ちで見ていく.

ちょっと上を見ると

r = refresh_one(controller, p, a, b, iteration, depth + 1, &child);

child はここで設定されてそう. refresh_one() を見ていく

static int refresh_one(
                const char *controller,
                const char *path,
                Hashmap *a,
                Hashmap *b,
                unsigned iteration,
                unsigned depth,
                Group **ret) {

        _cleanup_closedir_ DIR *d = NULL;
        Group *ours;
        int r;

        assert(controller);
        assert(path);
        assert(a);

        if (depth > arg_depth)
                return 0;

        r = process(controller, path, a, b, iteration, &ours);
        if (r < 0)
                return r;
…
        if (ret)
                *ret = ours;

        return 1;
}

"*ret = ours" されてて, "process(controller, path, a, b, iteration, &ours);" されてるけど, ours が初期化されていない. ただ "if (r < 0)" でエラー処理はされてて, その場合 ret が更新されないのが気になる.

process() を見てみると, どうもエラーが起きているのに 0 を返しているパスがある. この場合, ours は変更されない.

        } else if (streq(controller, "blkio") && cg_unified() <= 0) {
                _cleanup_fclose_ FILE *f = NULL;
                _cleanup_free_ char *p = NULL;
                uint64_t wr = 0, rd = 0;
                nsec_t timestamp;

                r = cg_get_path(controller, path, "blkio.io_service_bytes", &p);
                if (r < 0)
                        return r;

                f = fopen(p, "re");
                if (!f) {
                        if (errno == ENOENT)
                                return 0;
                        return -errno;
                }

ここで2つの論点がある. 1つはこの 0 を返しているのはどういうパスか, もう1つは ours をどこで NULL にするのが適切であるか.

前記のコードは "blkio.io_service_bytes" というファイルを開いている. これは cgroup の blkio tree で cgroup ごとの IO 量の統計を取得するファイルである. このファイルはIOスケジューラが cfq でなければ存在しない. そのため cfq をカーネルに組み込んでいないシステムでは ours が変更されずに process() から返っていくということになる.

  1. process() の先頭で NULL にする
  2. process() の return 部分で NULL にする
  3. refresh_one() で NULL 初期化する

systemd ではエラーの場合, "call-by-reference variables" を書きかえるな, という Coding Style なので*1, 1番はよろしくない.

2番は変更が面倒だけど, 正しい動作をする. 3番はシンプルだし, ここではうまく動くのだが全体的にはよくないところがある.

そもそも process() は refresh_one() 以外からも呼ばれうる. そして呼び出し側は refresh_one() が 0 を返してきた時, 返ってきたものが NULL か, 正しい Group を指していることを期待している. そうすると, 3番の修正だけでは不十分だということが言える.




って, 最後のはこれを書いてて気がついたんだけど, やっぱり面倒だしもう3番で PR 投げてしまったし, とりあえずね, 放置.

github.com