Hi,
On 16/12/2025 at 14:40:41 +01, Peter Suti <[email protected]>
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 <[email protected]>
Thanks,
Miquèl
