fix(uavcan): block firmware update while armed and arming while updating by Phil-Engljaehringer · Pull Request #28089 · PX4/PX4-Autopilot · GitHub

Solved Problem

UAVCAN firmware updates could be initiated while the vehicle was armed, causing nodes to reboot into their bootloader mid-flight. Conversely, there was no mechanism to prevent arming while a firmware update was already in progress, which could leave safety-critical nodes in a non-operational state during flight.

Solution

Two complementary protections are added:

1. No update initiation while armed

FirmwareVersionChecker::shouldRequestFirmwareUpdate() returns false immediately when the vehicle is armed. This prevents any node from being queued for a BeginFirmwareUpdate request during flight. The armed state is propagated from UavcanNode via actuator_armed uORB subscription → UavcanServers::setArmed() → FirmwareVersionChecker::setArmed().

2. Arming blocked while update is in progress

FirmwareVersionChecker tracks nodes under update in a dynamically-sized linked list (List<UpdatingNode *>) keyed by the node's 128-bit hardware unique ID, removing any fixed cap on the number of concurrently updating nodes. A node is added when shouldRequestFirmwareUpdate() returns true and removed when it returns false (CRCs match after reboot → update complete). Dynamic allocation is used intentionally here: firmware updates only occur before arming, so nodes are added and removed exclusively during the pre-arm phase. This state is published via a new uavcan_firmware_update uORB topic — only when pending_updates changes state, to avoid unnecessary bus traffic — and consumed by SystemChecks::checkAndReport() in commander, which blocks arming with a preflight failure while any update is pending.

Changelog Entry

Feature: UAVCAN firmware updates are now blocked while armed, and arming is blocked while a UAVCAN node firmware update is in progress.

Alternatives

Blocking the BeginFirmwareUpdate RPC in handleTimerEvent() while armed was considered. This was rejected because it would cause the node to be added to internal tracking structures without the update ever completing, potentially blocking arming indefinitely after disarm.
Using node ID instead of hardware unique ID for update tracking was considered. Rejected in favour of unique ID since it is a more stable identifier that does not depend on the dynamic node ID allocator state.
A fixed-size array was initially used for tracking updating nodes, capping concurrent updates at 8. This was replaced with an intrusive linked list (List<UpdatingNode *>) to remove that limit.

Test Coverage

Manual: verified on bench with a UAVCAN node with mismatched firmware → arming is blocked until update completes, and no update is sent while armed.

Context

The update lifecycle tracked by _updating_nodes covers the full path: node detected (CRC mismatch) → BeginFirmwareUpdate sent and confirmed → node in bootloader downloading firmware → node reboots with new firmware (CRC match). Arming is blocked for the entire duration.

Note: if a node with outdated firmware is already online when the vehicle disarms, it will not be detected until the node next reboots or reconnects. This is intentional — no forced re-query on disarm.

🔎 FLASH Analysis

px4_fmu-v5x [Total VM Diff: 896 byte (0.04 %)]

    FILE SIZE        VM SIZE    
 --------------  -------------- 
  +0.0%    +896  +0.0%    +896    .text
     +69%    +196   +69%    +196    uavcan_posix::FirmwareVersionChecker::shouldRequestFirmwareUpdate()
    +7.8%    +120  +7.8%    +120    UavcanNode::Run()
    +0.1%    +104  +0.1%    +104    g_cromfs_image
     +13%     +92   +13%     +92    SystemChecks::checkAndReport()
    +0.1%     +92  +0.1%     +92    [section .text]
   -99.8%     +58 -99.8%     +58    [34 Others]
    +1.0%     +40  +1.0%     +40    Commander::handle_command()
    +4.0%     +40  +4.0%     +40    UavcanNode::UavcanNode()
     +27%     +36   +27%     +36    SystemChecks::SystemChecks()
    +212%     +34  +212%     +34    uavcan_posix::FirmwareVersionChecker::~FirmwareVersionChecker()
    +0.2%     +32  +0.2%     +32    uORB::compressed_fields
    +2.3%     +28  +2.3%     +28    Commander::dataLinkCheck()
    +5.1%     +28  +5.1%     +28    UavcanNode::~UavcanNode()
    [NEW]     +16  [NEW]     +16    __orb_uavcan_firmware_update
    +5.4%     +12  +5.4%     +12    Commander::landDetectorUpdate()
     +17%     +12   +17%     +12    SystemChecks::updateParamsImpl()
    +1.6%      +8  +1.6%      +8    Commander::arm()
    +2.2%      +8  +2.2%      +8    Commander::disarm()
    -3.6%      -8  -3.6%      -8    Commander::checkForMissionUpdate()
    -2.4%     -20  -2.4%     -20    HealthAndArmingChecks::~HealthAndArmingChecks()
    -6.4%     -32  -6.4%     -32    uavcan::Array<>::push_back()
  +0.0%    +938  [ = ]       0    .debug_abbrev
  +0.1%    +112  [ = ]       0    .debug_aranges
  +0.1%    +332  [ = ]       0    .debug_frame
  +0.1% +25.1Ki  [ = ]       0    .debug_info
  +0.1% +3.51Ki  [ = ]       0    .debug_line
    +150%      +3  [ = ]       0    [Unmapped]
    +0.1% +3.51Ki  [ = ]       0    [section .debug_line]
  +0.1% +2.85Ki  [ = ]       0    .debug_loclists
  +0.1%    +370  [ = ]       0    .debug_rnglists
  +0.1% +4.19Ki  [ = ]       0    .debug_str
  -1.2%      -3  [ = ]       0    .shstrtab
  -0.0%     -57  [ = ]       0    .strtab
    [DEL]     -11  [ = ]       0    CSWTCH.799
    [NEW]     +11  [ = ]       0    CSWTCH.801
    +0.1%     +27  [ = ]       0    [section .strtab]
   -20.0%     -16  [ = ]       0    ___ZN3Ekf20controlGravityFusionERKN9estimator9imuSampleE_veneer
    +100%     +16  [ = ]       0    __memcpy_veneer
    [NEW]     +29  [ = ]       0    __orb_uavcan_firmware_update
    -7.0%    -113  [ = ]       0    uavcan::Array<>::push_back()
  +0.0%     +48  [ = ]       0    .symtab
    [DEL]     -32  [ = ]       0    CSWTCH.799
    [NEW]     +32  [ = ]       0    CSWTCH.801
   -14.3%     -16  [ = ]       0    Commander::Commander()
    +100%     +16  [ = ]       0    Commander::checkForMissionUpdate()
   -25.0%     -16  [ = ]       0    Commander::manualControlLossModeSwitch()
    +100%     +16  [ = ]       0    Commander::offboardControlCheck()
     +33%     +16  [ = ]       0    Commander::updateParamsImpl()
   -50.0%     -32  [ = ]       0    EKFGSF_yaw::fuseVelocity()
   -50.0%     -16  [ = ]       0    HealthAndArmingCheckBase::updateParams()
   -25.0%     -32  [ = ]       0    RcvTopicsPubs::init()
   -25.0%     -16  [ = ]       0    SystemChecks::checkAndReport()
     +20%     +16  [ = ]       0    SystemChecks::~SystemChecks()
     +67%     +32  [ = ]       0    UavcanNode::UavcanNode()
   -102.1%     -48  [ = ]       0    [13 Others]
    +0.5%     +64  [ = ]       0    [section .symtab]
   -40.0%     -32  [ = ]       0    ___ZN3Ekf20controlGravityFusionERKN9estimator9imuSampleE_veneer
     +33%     +16  [ = ]       0    ___ZNK3px46atomicIbE4loadEv_veneer
   -25.0%     -16  [ = ]       0    ____errno_veneer
     +50%     +16  [ = ]       0    __dq_rem_veneer
     +67%     +32  [ = ]       0    __memcpy_veneer
    [NEW]     +48  [ = ]       0    __orb_uavcan_firmware_update
   +39% +3.12Ki  [ = ]       0    [Unmapped]
  +0.1% +41.4Ki  +0.0%    +896    TOTAL

px4_fmu-v6x [Total VM Diff: 800 byte (0.04 %)]

    FILE SIZE        VM SIZE    
 --------------  -------------- 
  +0.0%    +800  +0.0%    +800    .text
     +69%    +196   +69%    +196    uavcan_posix::FirmwareVersionChecker::shouldRequestFirmwareUpdate()
    +0.1%    +136  +0.1%    +136    g_cromfs_image
    +7.8%    +120  +7.8%    +120    UavcanNode::Run()
     +13%     +92   +13%     +92    SystemChecks::checkAndReport()
    +0.1%     +88  +0.1%     +88    [section .text]
    +1.0%     +40  +1.0%     +40    Commander::handle_command()
    +4.0%     +40  +4.0%     +40    UavcanNode::UavcanNode()
     +27%     +36   +27%     +36    SystemChecks::SystemChecks()
    +212%     +34  +212%     +34    uavcan_posix::FirmwareVersionChecker::~FirmwareVersionChecker()
    +0.2%     +32  +0.2%     +32    uORB::compressed_fields
    +2.3%     +28  +2.3%     +28    Commander::dataLinkCheck()
    +5.1%     +28  +5.1%     +28    UavcanNode::~UavcanNode()
    [NEW]     +16  [NEW]     +16    __orb_uavcan_firmware_update
    +5.4%     +12  +5.4%     +12    Commander::landDetectorUpdate()
     +17%     +12   +17%     +12    SystemChecks::updateParamsImpl()
    +1.6%      +8  +1.6%      +8    Commander::arm()
    -3.6%      -8  -3.6%      -8    Commander::checkForMissionUpdate()
    -2.4%     -20  -2.4%     -20    HealthAndArmingChecks::~HealthAndArmingChecks()
    -4.7%     -20  -4.7%     -20    param_reset_specific
    -6.4%     -32  -6.4%     -32    uavcan::Array<>::push_back()
   -100.1%     -38 -100.1%     -38    [56 Others]
  +0.0%    +938  [ = ]       0    .debug_abbrev
  +0.1%    +112  [ = ]       0    .debug_aranges
  +0.1%    +312  [ = ]       0    .debug_frame
  +0.1% +24.9Ki  [ = ]       0    .debug_info
  +0.1% +3.42Ki  [ = ]       0    .debug_line
    +250%      +5  [ = ]       0    [Unmapped]
    +0.1% +3.41Ki  [ = ]       0    [section .debug_line]
  +0.1% +2.65Ki  [ = ]       0    .debug_loclists
  +0.1%    +354  [ = ]       0    .debug_rnglists
   -33.3%      -1  [ = ]       0    [Unmapped]
    +0.1%    +355  [ = ]       0    [section .debug_rnglists]
  +0.1% +4.19Ki  [ = ]       0    .debug_str
  +0.4%      +1  [ = ]       0    .shstrtab
  -0.0%     -57  [ = ]       0    .strtab
    [DEL]     -11  [ = ]       0    CSWTCH.799
    [NEW]     +11  [ = ]       0    CSWTCH.801
    +0.1%     +27  [ = ]       0    [section .strtab]
    [NEW]     +29  [ = ]       0    __orb_uavcan_firmware_update
    -7.0%    -113  [ = ]       0    uavcan::Array<>::push_back()
  +0.0%     +48  [ = ]       0    .symtab
    [DEL]     -32  [ = ]       0    CSWTCH.799
    [NEW]     +32  [ = ]       0    CSWTCH.801
   -14.3%     -16  [ = ]       0    Commander::Commander()
    +100%     +16  [ = ]       0    Commander::checkForMissionUpdate()
   -25.0%     -16  [ = ]       0    Commander::manualControlLossModeSwitch()
    +100%     +16  [ = ]       0    Commander::offboardControlCheck()
     +33%     +16  [ = ]       0    Commander::updateParamsImpl()
    +100%     +16  [ = ]       0    ConstLayer::containedAsBitset()
    +100%     +16  [ = ]       0    ConstLayer::contains()
   -33.3%     -16  [ = ]       0    ConstLayer::store()
     +25%     +16  [ = ]       0    DynamicSparseLayer::DynamicSparseLayer()
    +100%     +32  [ = ]       0    EKFGSF_yaw::fuseVelocity()
   -50.0%     -16  [ = ]       0    HealthAndArmingCheckBase::updateParams()
   -25.0%     -32  [ = ]       0    RcvTopicsPubs::init()
   -25.0%     -16  [ = ]       0    SystemChecks::checkAndReport()
     +20%     +16  [ = ]       0    SystemChecks::~SystemChecks()
     +25%     +16  [ = ]       0    UavcanNode::UavcanNode()
   -106.9%     -80  [ = ]       0    [13 Others]
    +0.1%     +16  [ = ]       0    [section .symtab]
    [NEW]     +48  [ = ]       0    __orb_uavcan_firmware_update
     +50%     +16  [ = ]       0    _buffer
   +72% +3.22Ki  [ = ]       0    [Unmapped]
  +0.1% +40.9Ki  +0.0%    +800    TOTAL

Updated: 2026-08-10T09:55:38

Claude review on behalf of @dakejahl

Blocker — _updating_nodes insert does not de-dupe (firmware_version_checker.hpp / shouldRequestFirmwareUpdate)
On rv == true the unique ID is written into the first empty slot with no “already present” check. After BeginFirmwareUpdate, the same hardware typically reappears in the bootloader (software_version 0.0 → rv true again via the existing 0.0 / zero-CRC path) and consumes another slot. One unstable node can fill all 8 entries with the same ID. Check for an existing match before insert (and only claim a free slot if absent).

Blocker — table full still starts an update but stops tracking it
If no free slot remains, the function still returns true, so FirmwareUpdateTrigger queues BeginFirmwareUpdate, but hasUpdatingNodes() can stay false. That allows arming while a node is being rebooted into the bootloader — the failure mode this PR is meant to close. Either refuse the update when it cannot be tracked (return false if insert fails), or set a sticky “update in progress / overflow” flag that keeps pending_updates true until cleared safely.

Concern — no clear on update failure / node loss → permanent arm deny
IDs are only removed when a later shouldRequestFirmwareUpdate returns false (CRC match / no image). That requires another GetNodeInfo (restart, reconnect, or invalidateAll). A node that dies mid-update leaves a stuck entry and blocks arming until FMU reboot, with no timeout and no operator recovery. Add a bounded lifetime (or clear when the node has been OFFLINE for N seconds / when servers give up), and surface enough info that the preflight message is actionable.

Concern — uavcan_firmware_update published every Run (~3 ms) (uavcan_main.cpp)
Unconditional publish whenever _servers != nullptr is far above the rate commander needs. Publish on change (optional low-rate refresh) instead of every cycle.

Nit — UavcanFirmwareUpdate.msg comment
Comment says “awaiting a BeginFirmwareUpdate request”, but the table is held through download/reboot until CRC matches. Align the field comment (and name if needed) with the full-lifecycle meaning.

Nit — title
[WIP]: Ufw prevent update/arming... fails the PR Title check; use Conventional Commits with a scope when this leaves WIP (e.g. fix(uavcan): block firmware update while armed and arming while updating).

Claude review on behalf of @dakejahl

Dedupe, the fixed cap, the publish rate, and the msg comment are all addressed. The "no clear on node loss" concern is still open, and the rewrite introduces two new defects.

Blocker — zero unique ID aliases two nodes to one entry (firmware_version_checker.hpp, shouldRequestFirmwareUpdate)
HardwareVersion.uavcan states: "All zeros is not a valid UID. If filled with zeros, assume that the value is undefined." Two such nodes dedupe into a single list entry. When the first completes, the removal loop deletes that shared entry, hasUpdatingNodes() returns false, and arming is allowed while the second node is still in its bootloader — the exact failure this PR closes.

Blocker — no recovery path; a lost node grounds the vehicle
Entries clear only via a successful GetNodeInfo returning rv == false. handleNodeInfoUnavailable() and handleNodeStatusChange(MODE_OFFLINE) remove from the trigger's pending_nodes_ but never reach the checker. shouldRetryFirmwareUpdate() returns true unconditionally (// TODO: Limit the number of attempts per node), so a node that keeps NAKing retries forever and never clears. There is no operator override: Commander.cpp:1109 passes cmd.from_external || !forced, so a GCS force-arm (21196) still runs preflight checks. uavcan update only helps if the node is still present and healthy. Net: a node that browns out or is unplugged during preflight blocks arming until the FMU is rebooted.

Recommended fix for both — key by NodeID in a fixed bitset, clear on offline
uavcan::Bitset<128> (uavcan/util/bitset.hpp, 16 bytes, already vendored) keyed by node ID: set/clear is idempotent so the dedupe walk goes away, no allocation, no UID collision, much less flash. Clear the bit when the node goes offline — UavcanNode already owns _node_status_monitor (forEachNode()/getNodeStatus()) and publishes per-node mode including MODE_SOFTWARE_UPDATE. Re: the rationale in the description — the trigger's own pending_nodes_, which drives the actual BeginFirmwareUpdate, is keyed by NodeID and re-keyed on the same events, so node ID is exactly as stable as the mechanism being tracked. This also puts the lifecycle state in UavcanServers/UavcanNode where node status is visible, leaving the checker a pure "does this node need an update?" predicate.

Concern — unchecked new, silent failure (firmware_version_checker.hpp)
memcpy(new_node->unique_id, …) runs before any null check, in both branches. NuttX libcxxmini's operator new is lib_malloc() with a DEBUGASSERT that compiles out in release, so it returns nullptr. List::add(nullptr) then silently no-ops, so OOM yields a null deref followed by an untracked in-progress update. uavcan_main.cpp:446 and :624 null-check new in this same module.

Nits

~UavcanNode deletes _servers without publishing pending_updates = false; uavcan stop leaves the latch stuck true and arming blocked.
uavcan_firmware_update isn't in logged_topics.cpp (dronecan_node_status is) — worth logging, since this topic is the answer to "why wouldn't it arm".
The message carries no node id/count, so the preflight message can't name which node is blocking.
actuator_armed.prearmed isn't covered by the gate; prearm enables outputs on FW/VTOL, so armed || prearmed is the stricter condition.

Claude review on behalf of @dakejahl

Zero-UID guard is placed wrong (firmware_version_checker.hpp:120-125)
The early return false sits ahead of the _updating_nodes bookkeeping, so a node with an all-zeros unique ID is silently skipped for firmware update and never blocks arming. A node that can't be identified is the case you'd most want to hold arming on, and this does the opposite. It's also an unannounced behaviour change: those nodes were updated before this PR. If the UID is unusable, either keep the node out of the update path but still latch pending_updates, or drop the guard and key the tracking off node ID.

Unchecked new (firmware_version_checker.hpp:186-188)
memcpy(new_node->unique_id, …) runs before any null check. NuttX libcxxmini's operator new is lib_malloc() with a DEBUGASSERT that compiles out in release, so it returns nullptr; List::add(nullptr) then silently no-ops, giving a null deref followed by an untracked in-progress update. uavcan_main.cpp:446 and :624 null-check new in this same module.

prearmed not covered (uavcan_main.cpp:758)
The gate uses actuator_armed.armed only. Prearm enables actuator outputs on FW/VTOL, so armed || prearmed is the stricter condition.

Nits

The /* EVENT */ block in systemCheck.cpp is empty. Every other circuit-breaker-gated check in that file carries an @description with <profile name="dev">This check can be configured via <param>…</param></profile>, so nothing currently surfaces CBRK_UAVCAN_FW to the operator in QGC or the docs.
CBRK_UAVCAN_FW_KEY = 5318008 ends up in the published parameter reference and in the param description text; worth a neutral value.

…ing (#28089) * feat: track Node updates * feat: prevent arming while updating * fix: used make format * fix: added dedup logic & use dynamic list * fix: cleaned up arming_blocker * feat: add circuit breaker for arming_blocker * chore: used make format * fix: fix some formating issues * chore: make format again * fix: implemented requested changes (cherry picked from commit 985aa09) Assisted-by: GitHub Copilot:gpt-6-astra Backport-note: Adapt to release/1.18 without importing unrelated NFS support.

Phil-Engljaehringer requested a review from alexcekay July 27, 2026 16:10

Phil-Engljaehringer force-pushed the ufw_update_arming branch from 79174b9 to 6659e6b Compare July 27, 2026 16:23

AlexKlimaj requested a review from dakejahl July 27, 2026 18:21

Phil-Engljaehringer changed the title [WIP]: Ufw prevent update/arming while armed/updating fix(uavcan): block firmware update while armed and arming while updating Jul 28, 2026

github-actions Bot added the kind:bug Something is broken or behaving incorrectly. label Jul 28, 2026

Phil-Engljaehringer force-pushed the ufw_update_arming branch 2 times, most recently from 2ced8ce to 8ae933e Compare July 28, 2026 12:51

Phil-Engljaehringer added 4 commits July 28, 2026 15:54

feat: track Node updates

8d7cdbb

feat: prevent arming while updating

f3b4e8c

fix: used make format

d30f82b

fix: added dedup logic & use dynamic list

b7598ef

Phil-Engljaehringer force-pushed the ufw_update_arming branch from 8ae933e to b7598ef Compare July 28, 2026 13:54

Phil-Engljaehringer added 3 commits August 4, 2026 17:50

Merge branch 'main' into ufw_update_arming

915ad6e

fix: cleaned up arming_blocker

3b6a691

feat: add circuit breaker for arming_blocker

110cf9e

github-actions Bot added the scope:parameters Parameter definitions, metadata, migration, or defaults. label Aug 5, 2026

Phil-Engljaehringer added 3 commits August 5, 2026 15:33

chore: used make format

8a96c5b

fix: fix some formating issues

0699533

chore: make format again

e810568

fix: implemented requested changes

0ff18bd

dakejahl approved these changes Aug 10, 2026

View reviewed changes

dakejahl merged commit 985aa09 into PX4:main Aug 10, 2026
76 checks passed

mrpollo mentioned this pull request Sep 9, 2026

fix(release): backport safety and reliability fixes to v1.18 #28649

Merged

fix(uavcan): block firmware update while armed and arming while updating by Phil-Engljaehringer · Pull Request #28089 · PX4/PX4-Autopilot · GitHub

fix(uavcan): block firmware update while armed and arming while updating - #28089

fix(uavcan): block firmware update while armed and arming while updating#28089
dakejahl merged 11 commits into
PX4:mainfrom
Phil-Engljaehringer:ufw_update_arming

Phil-Engljaehringer commented Jul 27, 2026 •
edited
Loading

Uh oh!

Uh oh!

github-actions Bot commented Jul 27, 2026 •
edited
Loading

Uh oh!

Uh oh!

dakejahl commented Jul 27, 2026

Uh oh!

dakejahl commented Jul 28, 2026 •
edited
Loading

Uh oh!

Uh oh!

dakejahl commented Aug 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fix(uavcan): block firmware update while armed and arming while updating by Phil-Engljaehringer · Pull Request #28089 · PX4/PX4-Autopilot · GitHub

fix(uavcan): block firmware update while armed and arming while updating - #28089

fix(uavcan): block firmware update while armed and arming while updating#28089dakejahl merged 11 commits intoPX4:mainPX4/PX4-Autopilot:mainfrom Phil-Engljaehringer:ufw_update_armingPhil-Engljaehringer/PX4-Autopilot:ufw_update_armingCopy head branch name to clipboard

Conversation

Phil-Engljaehringer commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Solved Problem

Solution

Changelog Entry

Alternatives

Test Coverage

Context

Uh oh!

github-actions Bot commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔎 FLASH Analysis

Uh oh!

dakejahl commented Jul 27, 2026

Uh oh!

dakejahl commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dakejahl commented Aug 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fix(uavcan): block firmware update while armed and arming while updating#28089
dakejahl merged 11 commits into
PX4:mainfrom
Phil-Engljaehringer:ufw_update_arming

Phil-Engljaehringer commented Jul 27, 2026 •
edited
Loading

github-actions Bot commented Jul 27, 2026 •
edited
Loading

dakejahl commented Jul 28, 2026 •
edited
Loading