ipc_pset_signal ignores the kqueue in-flux protocol that knote() implements

クローズ
  • 何か問題がありますか？
#166 コメント 1 件 リアクション 0 件 担当者 0 名 GitHub で見る

メンテナーはふだん 1 日以内に返信

まだ誰も着手していません。

評価

難易度
4/5
見積もり時間
3〜5日
初心者へのやさしさ
45/100
issue の種類
バグ
明瞭さ
明確に書かれている
活発さ
静か
技術スタック
c
領域
operating-systems

調査の方向性

Read sys/kern/kern_event.c, especially knote(), the kqueue_scan() paths around lines 2220 and 2243, and ipc_pset_signal(). First instrument activations where kn_in_flux(kn) is set and KN_SCAN is clear, then reproduce the failure and check the counter. If proceeding with the fix, preserve the KN_SCAN exception and verify the in-flux guard prevents unsafe enqueueing.

索引モデルが issue の本文から書いたものです。

説明

area:mach

Summary

ipc_pset_signal() hand-rolls knote activation and does not implement the
in-flux protocol that FreeBSD's own knote() implements explicitly. It can
therefore activate and enqueue a knote that the kqueue core has taken out of
the queue and marked in flux — including one that is being dropped and freed.

Found by reading sys/kern/kern_event.c from the base tree (stock
releng/15.1; no patch in patches/ touches it).

The protocol being violated

knote() skips in-flux knotes, and the comment says exactly why the one
exception is safe:

2585	SLIST_FOREACH_SAFE(kn, &list->kl_list, kn_selnext, tkn) {
2586		kq = kn->kn_kq;
2587		KQ_LOCK(kq);
2588		if (kn_in_flux(kn) && (kn->kn_status & KN_SCAN) == 0) {
2589			/*
2590			 * Do not process the influx notes, except for
2591			 * the influx coming from the kq unlock in the
2592			 * kqueue_scan().  In the later case, we do
2593			 * not interfere with the scan, since the code
2594			 * fragment in kqueue_scan() locks the knlist,
2595			 * and cannot proceed until we finished.
2596			 */
2597			KQ_UNLOCK(kq);
2598		}

ipc_pset_signal() has no equivalent. It tests only KN_QUEUED | KN_DISABLED:

(kn)->kn_status |= KN_ACTIVE;
if (((kn)->kn_status & (KN_QUEUED | KN_DISABLED)) == 0)
        mach_knote_enqueue(kn);

Why it cannot simply call knote()

filt_machport() has side effects — it performs the receive into
current_thread()'s ith_* state. Called from knote() on the sender's
thread it would deliver the notification into the wrong thread entirely. That
is presumably why the activation was open-coded. The influx check was not
carried across with it.

Worst case

kqueue_scan()'s EV_ONESHOT path puts a knote in flux without KN_SCAN
and then frees it:

2220	} else if ((kn->kn_flags & EV_ONESHOT) == EV_ONESHOT) {
2221		kn->kn_status &= ~KN_QUEUED;
2222		kn_enter_flux(kn);
2223		kq->kq_count--;
2224		KQ_UNLOCK(kq);
	...
2230		knote_drop(kn, td);

Between 2224 and the drop, ipc_pset_signal() may observe KN_QUEUED clear,
TAILQ_INSERT_TAIL the knote onto kq_head and bump kq_count — on a knote
that is then freed. filt_machport() sets EV_ONESHOT itself on both its
EV_EOF and translate-failure paths, so this is reachable.

A second shape: the zero-return path at 2243 clears KN_QUEUED and
decrements kq_count without a TAILQ_REMOVE, because the knote was
already unlinked when the scan dequeued it. A concurrent re-insertion by
ipc_pset_signal() leaves the knote physically on kq_head while marked
not-queued, with kq_count under-counted.

Relationship to the wedge

Not yet established, and should not be assumed. Every stage of the delivery
path has now measured clean (see #145), so an activation that is dropped or
mis-queued at the kqueue boundary is a live candidate — but three previous
candidates in this area looked equally sound and measured zero.

Instrument before fixing. Count activations landing on a knote with
kn_in_flux(kn) set and KN_SCAN clear — precisely the case knote()
skips. If that counter stays zero while the failure reproduces, this is a
latent correctness bug worth fixing on its own merits but not the wedge.

Fix

Add the same guard knote() uses, keeping the KN_SCAN exception, which is
safe here for the same documented reason: kqueue_scan() holds the knlist
lock across f_event, and ipc_pset_signal() takes that same lock
(ips_note_lock, exclusive via kn_sx_lock on the kqueue side, shared here),
so the two cannot interleave.

主要言語
C
スター
12
フォーク
3
平均マージ
1時間 39分
マージ済み PR（30日）
101

環境構築

このプロジェクトには開発コンテナ、Dockerfile、コントリビューションガイドがありません。まず README を読み、一般的な手順ははじめてのコントリビューションガイドを参照してください。

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

nextbsd/nextbsd-kernel のほかの issue

nextbsd/nextbsd-kernel の issue をすべて見る

似ている issue

C の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。