Linux 5.14.4のリリースして同日にregressionが報告されて次の日には5.14.5がリリースされてたんですが、これがどんなバグだったのかなというのを調べたメモです。
バグ報告の内容としては5.14.4でNextcloudっていうphpのwwebアプリケーションを実行するとハングアップするということでした。このときにbisectも行われていて、[PATCH 5.14 011/334] posix-cpu-timers: Force next expiration recalc after itimer resetがbad commitというところまで判明してました。スレッドではpatchの作者さんによる修正patchを送ったよというメールもありますが、5.1.4.5では関連するpatchのrevertにて対応されています。
5.14.5でrevertされたのは2つのpatchでした。5.14.5はこのregressionの修正のみのリリースでした。
まずbad commitのほうでこれはposix-cpu-timers: Force next expiration recalc after itimer resetですね。こちらはこのようなpatchでした。
diff --git a/kernel/time/posix-cpu-timers.c b/kernel/time/posix-cpu-timers.c index 517be7fd175e..a002685f688d 100644 --- a/kernel/time/posix-cpu-timers.c +++ b/kernel/time/posix-cpu-timers.c @@ -1346,8 +1346,6 @@ void set_process_cpu_timer(struct task_struct *tsk, unsigned int clkid, } } - if (!*newval) - return; *newval += now; }
もう一つはtime: Handle negative seconds correctly in timespec64_to_ns()です。こちらはこんな感じのpatchでした。
--- a/include/linux/time64.h +++ b/include/linux/time64.h @@ -25,7 +25,9 @@ struct itimerspec64 { #define TIME64_MIN (-TIME64_MAX - 1) #define KTIME_MAX ((s64)~((u64)1 << 63)) +#define KTIME_MIN (-KTIME_MAX - 1) #define KTIME_SEC_MAX (KTIME_MAX / NSEC_PER_SEC) +#define KTIME_SEC_MIN (KTIME_MIN / NSEC_PER_SEC) /* * Limits for settimeofday(): @@ -124,10 +126,13 @@ static inline bool timespec64_valid_sett */ static inline s64 timespec64_to_ns(const struct timespec64 *ts) { - /* Prevent multiplication overflow */ - if ((unsigned long long)ts->tv_sec >= KTIME_SEC_MAX) + /* Prevent multiplication overflow / underflow */ + if (ts->tv_sec >= KTIME_SEC_MAX) return KTIME_MAX; + if (ts->tv_sec <= KTIME_SEC_MIN) + return KTIME_MIN; + return ((s64) ts->tv_sec * NSEC_PER_SEC) + ts->tv_nsec; }
patchだけだとわかりにくいので全体を見てみましょう。。。まずはset_process_cpu_timer()から。revert後の5.14.5ではこんな関数です。
void set_process_cpu_timer(struct task_struct *tsk, unsigned int clkid, u64 *newval, u64 *oldval) { u64 now, *nextevt; if (WARN_ON_ONCE(clkid >= CPUCLOCK_SCHED)) return; nextevt = &tsk->signal->posix_cputimers.bases[clkid].nextevt; now = cpu_clock_sample_group(clkid, tsk, true); if (oldval) { /* * We are setting itimer. The *oldval is absolute and we update * it to be relative, *newval argument is relative and we update * it to be absolute. */ if (*oldval) { if (*oldval <= now) { /* Just about to fire. */ *oldval = TICK_NSEC; } else { *oldval -= now; } } if (!*newval) return; *newval += now; } /* * Update expiration cache if this is the earliest timer. CPUCLOCK_PROF * expiry cache is also used by RLIMIT_CPU!. */ if (*newval < *nextevt) *nextevt = *newval; tick_dep_set_signal(tsk, TICK_DEP_BIT_POSIX_TIMER); }
↓の部分がpatchが変更する箇所ですね。
if (!*newval) return; *newval += now;
patchではif文のところを消して常にnewvalが指す値にnowの値を足すようになってます。元のコードではnewvalの指す値が0なら何もしないでreturnするんですが、patchではif文を消してるので常にその先が実行されるようになりますね。そうすると、今までは呼ばれることがなかったtick_dep_set_signal()が実行されるようになって本来送る必要のないシグナルを送ってしまうと修正patchのコミットメッセージにも書かれています。これがregressionとして現れてたんですかねぇ。
もう一つのrevertはtime: Handle negative seconds correctly in timespec64_to_ns()です。こちらの変更箇所はtimespec64_to_ns()です。これもrevert後のコードを見るとこんな感じになってます。
static inline s64 timespec64_to_ns(const struct timespec64 *ts) { /* Prevent multiplication overflow */ if ((unsigned long long)ts->tv_sec >= KTIME_SEC_MAX) return KTIME_MAX; return ((s64) ts->tv_sec * NSEC_PER_SEC) + ts->tv_nsec; }
patchでの変更は次のようになっています。
- /* Prevent multiplication overflow */ - if ((unsigned long long)ts->tv_sec >= KTIME_SEC_MAX) + /* Prevent multiplication overflow / underflow */ + if (ts->tv_sec >= KTIME_SEC_MAX) return KTIME_MAX; + if (ts->tv_sec <= KTIME_SEC_MIN) + return KTIME_MIN; +
timespec64構造体のtv_secはtime64_tでこれは__s64のtypedefとなってます。ということでtv_secは符号ありの64bit整数値です。ということで、コミットメッセージにも書いてますが、もともとはs64の値をunsigned long long(64bitの符号なし整数ですね)にキャストしてチェックしているので負の整数の場合にKTIME_MAXを返すことになりますと。で、patchでは符号ありの整数としてKTIME_SEC_MAXを超えたり、KTIME_SEC_MIN以下になる場合には上限値や下限値を返す感じにしています。ここだけ見るとそうだなと思うんですが、今までと返る値が変わることによる影響もあったんでしょうか。。。
いずれにせよ、5.14.5では2つのpatchのrevertが行われてregressionが修正されたということでした。
