Re: [PATCH 1/2] cmd: mtd: fix read failure on default full partition read with bad blocks

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi,

On 16/12/2025 at 14:40:41 +01, Peter Suti <peter.suti@streamunlimited.com> wrote:

> When performing an 'mtd read' on a NAND partition, the presence of bad
> blocks can cause the command to fail with error -22 (EINVAL) if the
> requested size is equal to (or close to) the partition size.
> When size is not provided, the whole partition is read by default.
>
> This issue occurs because the bad block skipping logic increments the
> physical read offset ('off') without decrementing the 'remaining' byte
> count. If enough bad blocks are skipped, the 'off' pointer eventually
> slides past the end of the partition boundary ('mtd->size') while
> 'remaining' is still non-zero. The subsequent call to 'mtd_read_oob'
> then fails its internal boundary checks.
>
> The Scenario:
>   Partition Size: 0x1200000
>   Bad Blocks:     1
>   Command:        mtd read swufit ${loadaddr}
>                   (Defaults to reading full 0x1200000 bytes)
>
>   1. U-Boot attempts to read 0x1200000 valid bytes.
>   2. It encounters the bad block and skips it (off += mtd->erasesize).
>   3. To satisfy the full request, it must read
>      (0x1200000 + mtd->erasesize) physical bytes.
>   4. The read pointer 'off' exceeds 'mtd->size'.
>   5. Command aborts with: "Read on swufit failed with error -22".
>
> Add a boundary check at the start of the read loop. If the read pointer
> 'off' reaches the end of the physical device, check if the user explicitly
> requested a size (argc >= 2).
>
> * If no size was specified (default read): Stop the read gracefully,
>   print a notice, and return success. We assume the user wants "all
>   available data" up to the physical limit.
> * If a size was explicitly specified: Continue standard execution. This
>   will result in an error (-22) as expected, since the specific requirement
>   cannot be met.
>
> It is physically impossible to read the full logical size of a partition
> if it contains bad blocks (as the bad blocks consume physical space
> required for the data). In this case, truncating the read is reasonable.
> For common use cases like loading FIT images (which often
> contain padding at the end), missing the trailing 0xFF data is acceptable,
> whereas failing the entire load operation is not.
>
> Also add some logging when bad blocks are encountered.
>
> NOTE: This changes the behavior of 'mtd read' when no size is provided.
>       Previously, it failed if the full read was impossible. Now, it
>       succeeds with a truncated buffer. The caller is responsible for
>       verifying data integrity (e.g., via fitImage hash verification).

Thanks for the improvement!

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thanks,
Miquèl