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() から返っていくということになる.
- process() の先頭で NULL にする
- process() の return 部分で NULL にする
- 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 投げてしまったし, とりあえずね, 放置.
