embedded-sdmmc-rs: u32 overflow in finding fat_offset from cluster

Also, note that any directory operation involves a linear directory walk, so ideally you shouldn’t have too many files in one directory. Perhaps create /0/1/2/0012345.dat or something.

OK, that all seems normal. I’ll try your example and see what is going wrong - perhaps it’s a bug when the root directory takes up more than one cluster?

big_dir.rs:

extern crate embedded_sdmmc;

mod linux;
use linux::*;

const FILE_TO_APPEND: &str = "README.TXT";

use embedded_sdmmc::{Error, Mode, VolumeIdx, VolumeManager};

fn main() -> Result<(), embedded_sdmmc::Error<std::io::Error>> {
    env_logger::init();
    let mut args = std::env::args().skip(1);
    let filename = args.next().unwrap_or_else(|| "/dev/mmcblk0".into());
    let print_blocks = args.find(|x| x == "-v").map(|_| true).unwrap_or(false);
    let lbd = LinuxBlockDevice::new(filename, print_blocks).map_err(Error::DeviceError)?;
    let mut volume_mgr: VolumeManager<LinuxBlockDevice, Clock, 8, 8, 4> =
        VolumeManager::new_with_limits(lbd, Clock, 0xAA00_0000);
    let mut volume = volume_mgr
        .open_volume(embedded_sdmmc::VolumeIdx(1))
        .unwrap();
    log::info!("Volume: {:?}", volume);
    let mut root_dir = volume.open_root_dir().unwrap();

    let mut file_num = 0;
    loop {
        file_num += 1;
        let file_name = format!("{}.da", file_num);
        let mut file = match root_dir.open_file_in_dir(
            file_name.as_str(),
            embedded_sdmmc::Mode::ReadWriteCreateOrTruncate,
        ) {
            Ok(f) => f,
            Err(err) => {
                log::error!("open file failed {:?}", err);
                break;
            }
        };
        log::info!("open file");
        let buf = b"hello world, from rust";
        let _ = file.write(&buf[..]).unwrap();

        log::info!("write file");
        drop(file);
        log::info!("close file");
    }

    Ok(())
}

$ zcat ./tests/disk.img.gz > disk.img
$ cargo run --example big_dir -- ./disk.img                     
warning: unused imports: `Mode`, `VolumeIdx`
 --> examples/big_dir.rs:8:29
  |
8 | use embedded_sdmmc::{Error, Mode, VolumeIdx, VolumeManager};
  |                             ^^^^  ^^^^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

warning: constant `FILE_TO_APPEND` is never used
 --> examples/big_dir.rs:6:7
  |
6 | const FILE_TO_APPEND: &str = "README.TXT";
  |       ^^^^^^^^^^^^^^
  |
  = note: `#[warn(dead_code)]` on by default

warning: `embedded-sdmmc` (example "big_dir") generated 2 warnings (run `cargo fix --example "big_dir"` to apply 1 suggestion)
    Finished dev [unoptimized + debuginfo] target(s) in 0.02s
     Running `target/debug/examples/big_dir ./disk.img`
thread 'main' panicked at /home/jonathan/Documents/github/embedded-sdmmc-rs/src/fat/volume.rs:207:34:
attempt to multiply with overflow
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Hey, I love a reproducer. Thanks! Let’s add an explicit check and look at a backtrace.

RUST_BACKTRACE=1 ./target/debug/examples/big_dir ./disk.img 
thread 'main' panicked at /home/jonathan/Documents/github/embedded-sdmmc-rs/src/fat/volume.rs:183:13:
next_cluster called on invalid cluster ClusterId(fffffffc)
stack backtrace:
   0: rust_begin_unwind
             at /rustc/79e9716c980570bfd1f666e3b16ac583f0168962/library/std/src/panicking.rs:597:5
   1: core::panicking::panic_fmt
             at /rustc/79e9716c980570bfd1f666e3b16ac583f0168962/library/core/src/panicking.rs:72:14
   2: embedded_sdmmc::fat::volume::FatVolume::next_cluster
             at ./src/fat/volume.rs:183:13
   3: embedded_sdmmc::fat::volume::FatVolume::write_new_directory_entry
             at ./src/fat/volume.rs:405:31
   4: embedded_sdmmc::volume_mgr::VolumeManager<D,T,_,_,_>::open_file_in_dir
             at ./src/volume_mgr.rs:518:45
   5: embedded_sdmmc::filesystem::directory::Directory<D,T,_,_,_>::open_file_in_dir
             at ./src/filesystem/directory.rs:150:17
   6: big_dir::main
             at ./examples/big_dir.rs:28:30
   7: core::ops::function::FnOnce::call_once
             at /rustc/79e9716c980570bfd1f666e3b16ac583f0168962/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

OK, so the problem is that FAT32 doesn’t live in a fixed place - the location is given by some metadata. So when you open the root directory, the directory holds the value 0xFFFF_FFFC. There was a bug in the write_new_directory_entry function where it would map that value to the real block on disk for the purposes of loading directory blocks, but it didn’t update the cluster number. So, when it went to find the next cluster it was trying to find the next cluster after 0xFFFF_FFFC, which obviously isn’t a valid cluster number.

The patch is pretty simple:

-                let mut first_dir_block_num = match dir.cluster {
+                let mut current_cluster = match dir.cluster {
-                    ClusterId::ROOT_DIR => self.cluster_to_block(fat32_info.first_root_dir_cluster),
+                    ClusterId::ROOT_DIR => Some(fat32_info.first_root_dir_cluster),
-                    _ => self.cluster_to_block(dir.cluster),
+                    _ => Some(dir.cluster),
                 };
-                let mut current_cluster = Some(dir.cluster);
+                let mut first_dir_block_num = self.cluster_to_block(dir.cluster);