backup: read board vendor/model from the right cJSON nodes

[widgetii]

Problem

do_upgrade() looks up the vendor and model items of the board-info object, then calls cJSON_GetStringValue() on binfo — the object itself — instead of on the item it just found:

cJSON *c_vendor = cJSON_GetObjectItem(binfo, "vendor");
if (c_vendor) {
    char *bstr = cJSON_GetStringValue(binfo);   // <- binfo, not c_vendor
    if (bstr)
        strcpy(board, bstr);
}
cJSON *c_model = cJSON_GetObjectItem(binfo, "model");
if (c_model) {
    char *bstr = cJSON_GetStringValue(binfo);   // <- same
    strcpy(board_id, bstr);                     // <- and unguarded
    ...
}

detect_board() returns an object, and cJSON_GetStringValue() returns NULL for anything that isn't a string, so bstr is NULL in both branches. Consequences:

• the vendor branch is NULL-guarded, so it silently does nothing;
• the model branch is not, so it runs strcpy(board_id, NULL).

ipctool upgrade therefore segfaults on any board whose detector supplies a model key — currently Ruision, Anjoy and Hankvision (src/boards/*.c). On boards without one it survives, but board_id stays empty, so the hardware U-Boot variable set from it further down is never written.

Fix

Read from c_vendor/c_model, guard both against NULL, and bound the copies with snprintf — these strings come from board detection (sysinfo files, directory names) and were being strcpy/strcat'ed into fixed 1024-byte buffers unchecked.

Behaviour is otherwise preserved, including board_id carrying the model alone.

Note for the maintainer

board is assembled here and then never read again. I left that as-is rather than quietly change what the composed string is supposed to mean — happy to drop it, or to switch hardware to the full "vendor model" string, if either was the original intent.

Testing

Compile-tested. I haven't run ipctool upgrade on a Ruision/Anjoy/Hankvision board — if someone can reproduce the crash on stock master and confirm it's gone here, that would close the loop.

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

PR Summary by Qodo

Fix board vendor/model cJSON access in upgrade path

🐞 Bug fix 🕐 10-20 Minutes

AI Description

• Fix vendor/model extraction by reading string values from the correct cJSON items.
• Prevent NULL dereference and bound board/board_id writes using snprintf.
• Ensure the U-Boot "hardware" env var is set when a model is provided.

Diagram

graph TD
  A["ipctool upgrade"] --> B["do_upgrade()"] --> C["detect_board()"] --> D["board-info JSON (cJSON)"] --> E["Read vendor/model strings"] --> F["Populate board + board_id"] --> G["Set U-Boot env: hardware"]

Loading

High-Level Assessment

The PR’s approach is the most direct and least disruptive: it fixes the incorrect cJSON_GetStringValue() target, adds the missing NULL guard, and hardens buffer writes with bounded snprintf while preserving existing semantics (board_id remains model-only). Alternatives like switching to explicit cJSON_IsString() + item->valuestring or introducing a shared JSON/string-copy helper were considered but don’t materially improve correctness for this narrowly scoped fix.

Files changed (1) +10 / -12

Bug fix (1) +10 / -12

backup.cFix vendor/model extraction and bound board string writes in do_upgrade() +10/-12

Fix vendor/model extraction and bound board string writes in do_upgrade()

• Corrects vendor/model retrieval by calling cJSON_GetStringValue() on the vendor/model items rather than the board-info object itself. Adds NULL-guards and replaces unbounded strcpy/strcat with snprintf-based bounded writes to prevent crashes and potential buffer overflow, while keeping board_id as the model string and preserving existing behavior.

src/backup.c

[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

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}