fix(can): close the race between the identity check and the read-only store
メンテナーはふだん 1 日以内に返信
関連するプルリクエストがすでにマージされています。
- #60 @Tichael による — マージ済み
評価
- 難易度
- 3/5
- 見積もり時間
- 1〜2日
- 初心者へのやさしさ
- 68/100
- issue の種類
- バグ
- 明瞭さ
- 明確に書かれている
- 活発さ
- 活発
- 技術スタック
- rust
- 領域
- embedded-iot
調査の方向性
Start by reading the implementations of set_can_read_only(), is_can_read_only(), and the identity confirmation and revocation functions mentioned in the issue. Then inspect the existing CAN tests and add the described interleaving test: confirm identity, unlock, clear only the identity flag, and assert the bus reads as read-only. Done when the test passes and an unconfirmed identity always reads as read-only.
索引モデルが issue の本文から書いたものです。
説明
Follow-up to #49.
Problem
set_can_read_only(false) checks the identity flag and then stores the read-only flag in two separate steps:
let enabled = enabled || crate::can_log::is_can_logging() || !is_car_identity_confirmed();
CAN_READ_ONLY.store(enabled, Ordering::Relaxed);
If revoke_car_identity() runs between those two steps, the order is:
set_can_read_only(false)reads identity = confirmed, soenabled = false.revoke_car_identity()clears the identity and storesCAN_READ_ONLY = true.set_can_read_only(false)storesCAN_READ_ONLY = false.
The bus ends up unlocked with no confirmed identity, which is what the gate is meant to prevent.
Is it reachable today?
No. Nothing calls set_can_read_only(false) yet, and embassy tasks on one executor only switch at await points. It becomes possible if these functions are ever called from an interrupt, a second executor (e.g. an interrupt-priority executor on ESP) or another thread on PC.
The CAN logging check doesn't have this gap: is_can_read_only() already ORs in is_can_logging(), so it's enforced when the flag is read.
Suggested fix
Treat the identity like logging: OR !is_car_identity_confirmed() into is_can_read_only(), so an unconfirmed identity always reads as read-only, whatever CAN_READ_ONLY holds. Alternatively, wrap the check and the store in critical_section::with (core-interface already depends on critical-section).
Add a test that confirms, unlocks, and then clears only the identity flag (simulating the interleaving) and asserts that is_can_read_only() returns true.
- 主要言語
- Rust
- スター
- 1
- フォーク
- 0
- 平均マージ
- 12時間 49分
- マージ済み PR（30日）
- 19
環境構築
このプロジェクトの開発コンテナを、あなたの GitHub アカウントでブラウザ上に起動します。
- Dockerfile・Docker Compose ファイルなし
- プルリクエストのテンプレートなし
- コントリビューションガイドなし
はじめの一歩
- issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
- 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
- リポジトリをフォークし、ブランチを切って変更します。
- issue 番号を参照したプルリクエストを送ります。
OpenCarControls/firmware のほかの issue
-
e-gmp enhancement
難易度 5/5 1週間以上 初心者へのやさしさ 25/100
OpenCarControls/firmware#75 ·
メンテナーはふだん 1 日以内に返信
-
enhancement
難易度 5/5 1週間以上 初心者へのやさしさ 25/100
OpenCarControls/firmware#74 ·
メンテナーはふだん 1 日以内に返信
-
enhancement
難易度 4/5 3〜5日 初心者へのやさしさ 45/100
OpenCarControls/firmware#73 ·
メンテナーはふだん 1 日以内に返信
-
enhancement
難易度 5/5 1週間以上 初心者へのやさしさ 35/100
OpenCarControls/firmware#72 ·
メンテナーはふだん 1 日以内に返信
-
e-gmp enhancement
難易度 5/5 1週間以上 初心者へのやさしさ 25/100
OpenCarControls/firmware#71 ·
メンテナーはふだん 1 日以内に返信
OpenCarControls/firmware の issue をすべて見る
似ている issue
-
bug easy good first issue
難易度 2/5 1〜3時間 初心者へのやさしさ 85/100
lacs-project/sysknife#659 ·
メンテナーはふだん 2 日以内に返信
-
documentation
難易度 2/5 1〜3時間 初心者へのやさしさ 70/100
registrystack/registry-stack#2431 ·
メンテナーはふだん 1 日以内に返信
-
contribution
難易度 2/5 1〜3時間 初心者へのやさしさ 85/100
メンテナーはふだん 1 日以内に返信
-
難易度 2/5 1〜3時間 初心者へのやさしさ 85/100
farion1231/cc-switch#8863 · コメント 1 件 ·
メンテナーはふだん 1 日以内に返信
-
難易度 1/5 1時間未満 初心者へのやさしさ 85/100
DioxusLabs/dioxus#6006 ·
メンテナーはふだん 1 日以内に返信