[BUG] Stable 2.1.x regression: CardReader::diveToFile() uses an unterminated dosSubdirname

Did you test the latest bugfix-2.1.x code?

No, but I will test it now!

Bug Description

The stable Marlin 2.1.x branch, including releases 2.1.2.7 and 2.1.2.8, appears to contain a regression in CardReader::diveToFile().

The affected code is:

const uint8_t len = name_end - atom_ptr; char dosSubdirname[len + 1]; strncpy(dosSubdirname, atom_ptr, len);

Since len is exactly the number of characters before the next /, the source string is not shorter than len. Therefore, strncpy() copies exactly len characters and does not append a null terminator.

As a result, dosSubdirname[len] remains uninitialized, but dosSubdirname is subsequently passed as a C string to functions such as:

sub->open(inDirPtr, dosSubdirname, O_READ)

This causes undefined and intermittent behavior when accessing files inside subdirectories.

The current bugfix-2.1.x branch already contains the correct implementation:

strlcpy(dosSubdirname, atom_ptr, len + 1);

Stable 2.1.x source:

https://github.com/MarlinFirmware/Marlin/blob/1cd56c4ccd483045eb5a92c99e3ad3b5ab1bea6d/Marlin/src/sd/cardreader.cpp#L967-L979

Current bugfix-2.1.x source:

https://github.com/MarlinFirmware/Marlin/blob/e2817dbec4256220845de5a6bf88174981209e4d/Marlin/src/sd/cardreader.cpp#L1218-L1230

Bug Timeline

This appears to be a regression introduced by commit:

https://github.com/MarlinFirmware/Marlin/commit/108f0b0cf55cd5acd840837bd8bd50a488ee4a56

That commit changed the safe version:

strlcpy(dosSubdirname, atom_ptr, len + 1);

back to:

strncpy(dosSubdirname, atom_ptr, len);

without restoring the required null terminator.

The need for explicit termination had already been discussed in PR #10925 in 2018, where the suggested implementation included:

strncpy(dosSubdirname, dirname_start, len); dosSubdirname[len] = 0;

https://github.com/MarlinFirmware/Marlin/pull/10925

The code was later converted to strlcpy() by PR #26513:

https://github.com/MarlinFirmware/Marlin/pull/26513

Bug Timeline

This appears to be a regression introduced by commit: https://github.com/MarlinFirmware/Marlin/commit/108f0b0cf55cd5acd840837bd8bd50a488ee4a56 That commit changed the safe version: strlcpy(dosSubdirname, atom_ptr, len + 1); back to: strncpy(dosSubdirname, atom_ptr, len); without restoring the required null terminator. The need for explicit termination had already been discussed in PR #10925 in 2018, where the suggested implementation included: strncpy(dosSubdirname, dirname_start, len); dosSubdirname[len] = 0; https://github.com/MarlinFirmware/Marlin/pull/10925 The code was later converted to strlcpy() by PR #26513: https://github.com/MarlinFirmware/Marlin/pull/26513

Expected behavior

Directory components extracted by CardReader::diveToFile() should always be valid null-terminated C strings.

Functions such as CardReader::fileExists(), file selection, directory traversal, and Power Loss Recovery validation should reliably locate files stored inside subdirectories.

Actual behavior

Operations involving paths inside directories can fail intermittently because the temporary directory name is not null-terminated.

In my case, this affected Power Loss Recovery. The recovery records correctly referenced:

/LAKIS/BOX1.GCO

The /PLR0 and /PLR1 records were present and valid, but Resume Print was not offered because the referenced G-code file was intermittently reported as missing by card.fileExists().

After adding only:

dosSubdirname[len] = '\0';

and flashing the firmware, the existing recovery records were immediately recognized and Resume Print appeared. Subsequent real power-loss tests consistently performed the Z raise, offered Resume Print, and resumed from the correct position.

Although Power Loss Recovery exposed the problem in this case, the defect is not PLR-specific. It can affect any feature that uses diveToFile() with a path containing one or more directory components.

Steps to Reproduce

Build Marlin 2.1.2.7 or 2.1.2.8.

Place a G-code file inside a directory on the SD card, for example:

/LAKIS/BOX1.GCO Use a feature that resolves the complete path through CardReader::diveToFile(), such as file selection or CardReader::fileExists(). Observe that resolving the file may fail intermittently.

Add:

dosSubdirname[len] = '\0';

immediately after strncpy(), rebuild, and repeat the test.

The directory path is then resolved reliably.

The failure may depend on the previous contents of the stack because the final byte of dosSubdirname is uninitialized.

Suggested Fix

Backport the existing bugfix-2.1.x implementation to the stable branch:

const uint8_t len = name_end - atom_ptr; char dosSubdirname[len + 1]; -strncpy(dosSubdirname, atom_ptr, len); +strlcpy(dosSubdirname, atom_ptr, len + 1);

Alternatively, the existing strncpy() call must be followed by:

dosSubdirname[len] = '\0';

Using the already-tested strlcpy() implementation from bugfix-2.1.x would appear to be the preferable fix.

Version of Marlin Firmware

2.1.2.7 also confirmed in 2.1.2.8

Printer model

Custom Cartesian printer (“Talos”)

Electronics

MKS TinyBee / ESP32

LCD/Controller

MKS Mini 12864 V3

Other add-ons

MKS UPS24V, MKS SERVO42C on X & Y axes

Bed Leveling

ABL Bilinear mesh

Your Slicer

Prusa Slicer

Host Software

None

Don't forget to include

• A ZIP file containing your Configuration.h and Configuration_adv.h.

Additional information & file uploads

Configuration.zip