Do not dereference the SD card metadata that may not be there

[MrScothh]

What happens

On maintenance-10.x, SITL dies with SIGSEGV the moment the Configurator opens the Blackbox tab. That tab is the only one that asks for MSP_SDCARD_SUMMARY, and the request is what kills it.

Thread 1 "inav_9.1.0_SITL" received signal SIGSEGV, Segmentation fault.
0x000000010043a61d in serializeSDCardSummaryReply (dst=0x7ffffc780) at src/main/fc/fc_msp.c:345
#1  mspFcProcessOutCommand (cmdMSP=79, ...) at src/main/fc/fc_msp.c:1353
#2  mspFcProcessCommand (...) at src/main/fc/fc_msp.c:5172
#3  mspSerialProcessReceivedCommand (...) at src/main/msp/msp_serial.c:494
#4  mspSerialProcessOnePort (...) at src/main/msp/msp_serial.c:568
#5  taskHandleSerial (...) at src/main/fc/fc_tasks.c:118

Reproduced on a clean maintenance-10.x (be5b352) with no other changes, at -O2 and at -O0, so it is not an optimiser artefact.

Why

serializeSDCardSummaryReply() decides three lines earlier that no card is present, and then asks the card about itself anyway:

if (!sdcard_isInserted()) {
    state = MSP_SDCARD_STATE_NOT_PRESENT;
}
...
sbufWriteU32(dst, sdcard_getMetadata()->numBlocks / 2);

sdcard_getMetadata() returns NULL while sdcardVTable is unset, and the vtable is bound only when the card is actually wanted:

bool sdcardNeeded = (blackboxConfig()->device == BLACKBOX_DEVICE_SDCARD);
sdcardNeeded = sdcardNeeded || terrainConfig()->terrainEnabled;
if (sdcardNeeded) {
    sdcardInsertionDetectInit();
    sdcard_init();
    afatfs_init();
}

So the pointer is NULL on any board that has SD card support compiled in and is not using it, which is the common case, and on SITL launched without --sdcard.

Why it has not been seen before

The line itself is old: it is the same in 9.1.0 and in master. Two things kept it quiet.

On a flight controller, address zero is mapped and readable, so the dereference returns four bytes of flash and the Configurator shows a nonsense card size in a field nobody reads when no card is fitted. Wrong, but harmless.

SITL did not compile this branch at all until d8c74f0, feat(SITL): simulated SD card backed by a host image file (--sdcard), which added USE_SDCARD and USE_SDCARD_SITL to the SITL target. Before that the #else path wrote five zeroes and touched no pointer. A hosted build does not map address zero, so from that commit onwards the same old defect is fatal rather than cosmetic.

The change

Read the pointer once, and report zero blocks when there is no card driver, which is what the rest of the reply already says.

Testing

SITL on Windows, gcc 15.2, built from be5b352 with and without the patch. The requests the Blackbox tab sends, in the order it sends them:

request	before	after
MSP_FEATURE	ok	ok
MSP_DATAFLASH_SUMMARY	ok	ok
MSP_SDCARD_SUMMARY	SIGSEGV	ok, 17 bytes
MSP2_COMMON_SETTING_INFO	not reached	ok
MSP2_COMMON_SETTING	not reached	ok
MSP2_BLACKBOX_CONFIG	not reached	ok

With the patch the Configurator's Blackbox tab opens against SITL and the simulator stays up.

On hardware the visible difference is the reported card size when no card is fitted: a random number before, zero after.

[qodo-code-review]

ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Tip of the day

💡 Did you know, you can enable the Remediation agent and Qodo fixes findings in a dedicated fix PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Guard missing SD card metadata in MSP summaries

🐞 Bug fix 🕐 Less than 10 minutes

AI Description

• Prevents MSP SD card summaries from dereferencing metadata when no driver is bound.
• Reports zero capacity for unused cards, keeping SITL and Configurator sessions alive.

Diagram

sequenceDiagram
    actor Config as Configurator
    participant MSP as MSP Handler
    participant Summary as SD Summary
    participant Driver as SD Driver
    Config->>MSP: MSP_SDCARD_SUMMARY
    MSP->>Summary: Serialize reply
    Summary->>Driver: Get metadata
    Driver-->>Summary: Metadata or null
    alt Metadata available
        Summary-->>MSP: Report block count
    else Driver unbound
        Summary-->>MSP: Report zero blocks
    end
    MSP-->>Config: 17-byte summary

Loading

High-Level Assessment

The localized null guard is the best approach because sdcard_getMetadata() explicitly returns null when no driver is bound. Inferring metadata availability from insertion or initialization state would duplicate driver state and be less robust, while changing the getter contract would have broader impact.

Files changed (1) +4 / -1

Bug fix (1) +4 / -1

fc_msp.cSafely serialize SD capacity without bound metadata +4/-1

Safely serialize SD capacity without bound metadata

• Reads the SD card metadata pointer once and checks it before accessing the block count. MSP_SDCARD_SUMMARY now reports zero total capacity when no card driver is bound, preventing SITL crashes and invalid hardware capacity values.

src/main/fc/fc_msp.c