sdcard: report what the card says about itself, and its remaining life - #221
Conversation
There is no SMART on SD. The CID and CSD say what the card was sold as and
nothing about its condition, which is why "is this card worn out" has had no
answer on a camera.
A few lines do implement one: a vendor register read with CMD56 (GEN_CMD),
carrying a percentage of rated life used. `ipctool sdcard` reads the identity
out of /sys and, where the card is one of those, the register too:
---
sdcard:
device: mmcblk0
name: WX32G
manfid: 0x000003
oemid: 0x5344
cid: 035344575833324780e3a829fd0193bb
health:
vendor: SanDisk / Western Digital
life_used_percent: 1
signature: DW250316
manufacturer: Western Digital
Note the last line. The CID has no brand field, and its manufacturer id 0x03
is registered to SanDisk -- which Western Digital has owned since 2016, so a
WD Purple is indistinguishable from a SanDisk by the CID alone. The register
is the only place the card says what is printed on it.
TWO THINGS IT DELIBERATELY WILL NOT DO.
It does not probe a card whose manufacturer id is not in the table. CMD56 with
an argument a card does not implement is not free: measured on the board
below, the data phase times out, the card raises its ERROR status bit, and the
NEXT command on the host fails once before the card clears it. Harmless to a
diagnostic run by hand; not something to hand a camera that is writing video.
And it does not decode a vendor it has not been read against. Shipping a
plausible guess at another vendor's layout would print a life figure nobody
measured, which is worse than printing nothing -- so an unknown card gets a
sentence saying the register was not read and why, rather than a silent gap.
Measured on a WD Purple QD101 32 GB in an hi3516av300 (HiSilicon himci, kernel
4.9.37), with the camera recording throughout: the read costs about 5 ms
including fork and exec, 50 consecutive reads all succeeded, and the recorder
saw no dropped fragments and no write or sync errors. The register embeds the
card's own CID, which matches /sys byte for byte -- that is what says the reply
belongs to this card rather than being a stale buffer.
The ioctl needs the whole-device node: the kernel refuses MMC_IOC_CMD on a
partition with EPERM, to keep one partition's commands out of its siblings.
linux/mmc/ioctl.h is not in every toolchain's sysroot so the request is spelled
out locally; the ABI is stable.
Builds clean for arm32-musl, arm32-gnueabi and arm64-musl; cYAML, reginfo and
longse tests pass, as does tools/test_pipeline.sh.
PR Summary by QodoAdd SD card identity and vendor health reporting
AI Description
Diagram
High-Level Assessment
Files changed (4)
|
Code Review by Qodo
1.
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
| for (size_t i = 0; i < sizeof(lay->sig) / sizeof(lay->sig[0]); i++) | ||
| if (lay->sig[i] && !memcmp(buf, lay->sig[i], strlen(lay->sig[i]))) | ||
| return true; |
There was a problem hiding this comment.
4. Wrong replies can become health data 🐞 Bug ≡ Correctness
sig_matches validates only the first two bytes against DS or DW, while the documented embedded card identifier at offset 0x195 is never compared with the sysfs identifier. Any successful response carrying that short prefix is consequently decoded as the selected card's life value even when it does not belong to that card.
Agent Prompt
## Issue description
Health data is accepted using only a two-byte prefix even though the register contains an embedded card identifier intended to prove that the reply belongs to the selected card.
## Fix Focus Areas
- src/sdcard.c[83-98]
- src/sdcard.c[172-178]
- src/sdcard.c[229-266]
## Recommended Fix
Parse the 32 hexadecimal CID characters read from sysfs into 16 bytes and compare them with the register bytes at offset `0x195` after the signature check. Reject the response without decoding health data when parsing fails or the identifiers differ.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Five from review, and the first one matters most because it undoes a safety claim the file made about itself. **A manufacturer id is not a safety gate.** 0x03 is SanDisk's, and Western Digital has shipped under it since buying them -- so it covers the WD Purple that implements the health register AND every consumer SanDisk that does not, which is most of the cards in these cameras. Gating the probe on it looked like a rule and probed almost everything, which is exactly the one-poisoned-command cost the file documents, spent on cards that were never going to answer. So the identity is free and always printed, and the register is read only when asked for with --health, with what that costs written into --help. What the vendor table still decides is decoding: a layout nobody has read against a real card is not guessed at. **The card is found, not assumed.** find_card() counted mmcblk0..3 and took the first device with a cid. It now walks /sys/block and insists on type "SD": the numbering follows probe order so a card can land anywhere, and a board with eMMC usually has it as mmcblk0 with the card behind it -- reporting the soldered part, let alone aiming a vendor command at it, is the wrong answer to "what is in the slot". **The reply has to belong to the card.** Two signature bytes are not much of a claim. The commit that added this said the embedded CID is what ties the 512 bytes to the card in the slot, and then did not check it. It does now, and reports `cid_echoed`. Searched for rather than read at a fixed offset, and reported rather than enforced: it was confirmed at 0x195 on one card, and a layout that puts it elsewhere should not have its health refused on the strength of one sample. **A failed read is not a register of zeroes.** --raw serialised the buffer whether or not anything came back, so an ioctl failure printed 512 bytes of made-up register. It is emitted only after a read that produced one. Checked on the WD Purple QD101 in an hi3516av300: the default prints identity and sends the card nothing, --health returns the register with `cid_echoed: true`, and --raw after a refused read prints no raw field. cYAML, reginfo, longse and test_pipeline.sh pass; arm32-musl and arm64-musl build clean; clang-format reports no replacements.
|
All five were right, and the first one undoes a claim the file made about 1. Some cards make later commands fail. The sharpest of the five. A manufacturer id cannot decide this, so nothing pretends it can. The identity 2 and 3. The wrong card, and cards above mmcblk3. Both real. 4. Wrong replies can become health data. A fair hit, and the gap was between 5. Failed reads look like zeroed registers. Correct. Checked on the WD Purple QD101 in an hi3516av300:
|
Follow-up to review finding 4. The corroboration was there but it was a boolean at the bottom of the block, which is exactly the kind of thing a reader skims past on the way to the number. When the reply does not carry this card's CID, the note beside life_used_percent now says so instead of describing what the figure means in general. Still not a gate, and deliberately. The signature is the vendor's own magic, so a reply that matches IS this vendor's health register; the only thing in doubt is whether it came from the card in the slot, and the ioctl was aimed at that card's own node. Refusing to print a figure on that basis would lose a good reading from any card whose layout puts its CID somewhere other than the one offset this has been read against. Reporting beats refusing when the uncertainty is about provenance rather than content -- and now it reports loudly.
|
Finding 4 — I have improved it but deliberately not gated on it, so here is the The corroboration was already computed; it was a boolean at the bottom of the Why it still prints the figure. The signature is the vendor's own magic, so Against that, gating has a concrete cost: the offset was confirmed on exactly So: report loudly, refuse nothing. If you would rather it hard-fail without the The other four are in 4cab0b9 and verified on the WD Purple QD101 in an |
1 participant
There is no SMART on SD. The CID and CSD say what a card was sold as and
nothing about its condition, so "is this card worn out" has had no answer on a
camera. A few lines — industrial and surveillance ones — do implement a vendor
health register, read with CMD56 (GEN_CMD), carrying a percentage of rated
life used.
ipctool sdcardreads the identity out of/sysand, where the card is one ofthose, the register too:
--jsonand--raw(the 512-byte register as hex) are there too.Look at the last line. The CID has no brand field, and its manufacturer id
0x03is registered to SanDisk — which Western Digital has owned since 2016,so a WD Purple is indistinguishable from a SanDisk by the CID alone. The
register is the only place the card says what is printed on it.
Two things it deliberately will not do
It does not probe a card whose manufacturer id is not in the table. CMD56
with an argument a card does not implement is not free. Measured on the board
below: the data phase times out (
-110), the card then raises its ERROR statusbit, and the next command on the host fails once (
-13,himci_cmd_done: The status of the card is abnormal) before the card clearsit. It self-heals — three follow-up CMD13s and a
ddall passed — so it isharmless to a diagnostic run by hand, but it is not something to hand a camera
that is writing video.
It does not decode a vendor it has not been read against. #833's survey
names SanDisk Industrial, WD Purple, Kingston Industrial, Apacer and Longsys,
each with its own argument and layout. I have one card. Shipping a plausible
guess at the other four would print a life figure nobody measured, which is
worse than printing nothing — so an unknown card gets a sentence saying the
register was not read and why, rather than a silent gap. The table is one
struct per vendor; adding one is a few lines plus a card to check it on.
Measured
WD Purple QD101 32 GB in an hi3516av300 (HiSilicon
himci, kernel 4.9.37),with the camera recording at 4 Mbit throughout:
binary, no failures
records_fsync_us_maxdid not moveThe register embeds the card's own CID at offset
0x195and it matches/sysbyte for byte. That is what says the reply belongs to this card rather than
being a stale buffer, and it is worth knowing the check exists.
Notes for review
MMC_IOC_CMDon a partition with
EPERM, to keep one partition's commands out of itssiblings — so this opens
/dev/mmcblk0, neverp1.linux/mmc/ioctl.his not in every toolchain's sysroot, so the request isspelled out locally. The ABI is stable.
the report runs on every bare
ipctool, and issuing an SD command therewould be a behaviour change for every camera rather than something asked for.
__arm__/__mips__/__aarch64__guards.is an ioctl. I could expose
layout_for()and the signature check to pin"never probe an unknown vendor", which is the one rule worth pinning — say
the word and I will add it.
Built and tested
arm32-musl, arm32-gnueabi and arm64-musl all build clean with no warnings from
the new file.
cYAML_test,reginfo_test,longse_testandtools/test_pipeline.shall pass.clang-formatreports no replacements.mips32 not built locally (no toolchain on this machine) — CI covers it.
Related
A latent bug in the HiSilicon
himcidriver found while establishing that thepassthrough is safe — one error path that never completes the MMC request,
which wedges the host — is OpenIPC/linux#57. It is not required for this to
work; nothing here triggers it.