Preserve 4 MSB in FAT32 entries and add more sanity checks (#13)

* Update compatibility maximum cluster size when sector size is larger than 512 bytes.

* Additional validation of values in FsInfo sector.
* next_free_cluster cannot be 0 or 1, as these are reserved clusters
* next_free_cluster must be a valid cluster (using BPB to validate)
* free_cluster_count must be possible value (using BPB to validate)

* Avoid data-loss edge-case on volumes over 138GB in size.

Specifically, if the volume has more
than 0x0FFF_FFF4 clusters, then the FAT
will include an entry for clusters that
are reserved values.  Specifically, cluster
number 0x0FFF_FFF7 is defined to mean
BAD_SECTOR, while numbers 0x0FFF_FFF8 ..
0x0FFF_FFFF are defined to mean end-of-chain.

This prevents these clusters from being part
of any valid cluster chain.  Therefore:
1. prevent setting these clusters to point to
   a valid next cluster
2. prevent reading these clusters as pointing
   to a valid next cluster

Instead, always read/write these FAT entries
to have next cluster value of BAD_SECTOR.

* Reduce noisy warnings on FAT32.

* The reserved bits in FAT entry must be preserved on update.

* Change the set() implementation for FAT32 entries to return an actual Err(),
when attempting to set values for cluster numbers that have special meaning.

src/fs.rs

@@ -15,7 +15,7 @@ use byteorder_ext::{ReadBytesExt, WriteBytesExt};
 use dir::{Dir, DirRawStream};
 use dir_entry::DIR_ENTRY_SIZE;
 use file::File;
-use table::{alloc_cluster, count_free_clusters, read_fat_flags, ClusterIterator};
+use table::{alloc_cluster, count_free_clusters, read_fat_flags, ClusterIterator, RESERVED_FAT_ENTRIES};
 use time::{TimeProvider, DEFAULT_TIME_PROVIDER};
 
 // FAT implementation based on:
@@ -167,12 +167,12 @@ impl BiosParameterBlock {
             // 32k is the largest value to maintain greatest compatibility
             // Many implementations appear to support 64k per cluster, and some may support 128k or larger
             // However, >32k is not as thoroughly tested...
-            warn!("fs compatibility: bytes_per_cluster value '{}' in BPB exceeds 32k, and thus may be incompatible with some implementations", bytes_per_cluster);
+            warn!("fs compatibility: bytes_per_cluster value '{}' in BPB exceeds '{}', and thus may be incompatible with some implementations", bytes_per_cluster, maximum_compatibility_bytes_per_cluster);
         }
 
         if bpb.reserved_sectors < 1 {
             return Err(Error::new(ErrorKind::Other, "invalid reserved_sectors value in BPB"));
-        } else if bpb.reserved_sectors != 1 {
+        } else if !bpb.is_fat32() && bpb.reserved_sectors != 1 {
             // Microsoft document indicates fat12 and fat16 code exists that presume this value is 1
             warn!("fs compatibility: reserved_sectors value '{}' in BPB is not '1', and thus is incompatible with some implementations", bpb.reserved_sectors);
         }
@@ -188,7 +188,6 @@ impl BiosParameterBlock {
             bpb.sectors_per_fat_32 = rdr.read_u32::<LittleEndian>()?;
             bpb.extended_flags = rdr.read_u16::<LittleEndian>()?;
             bpb.fs_version = rdr.read_u16::<LittleEndian>()?;
-            // TODO: Validate the only valid fs_version value is still zero, then check that here
             bpb.root_dir_first_cluster = rdr.read_u32::<LittleEndian>()?;
             bpb.fs_info_sector = rdr.read_u16::<LittleEndian>()?;
             bpb.backup_boot_sector = rdr.read_u16::<LittleEndian>()?;
@@ -323,10 +322,16 @@ impl FsInfoSector {
         }
         let free_cluster_count = match rdr.read_u32::<LittleEndian>()? {
             0xFFFFFFFF => None,
+            // Note: value is validated in FileSystem::new function using values from BPB
             n => Some(n),
         };
         let next_free_cluster = match rdr.read_u32::<LittleEndian>()? {
             0xFFFFFFFF => None,
+            0 | 1 => {
+                warn!("invalid next_free_cluster in FsInfo sector (values 0 and 1 are reserved)");
+                None            
+            },
+            // Note: other values are validated in FileSystem::new function using values from BPB
             n => Some(n),
         };
         let mut reserved2 = [0u8; 12];
@@ -486,6 +491,7 @@ impl<T: ReadWriteSeek> FileSystem<T> {
         let fat_sectors = bpb.fats as u32 * sectors_per_fat;
         let data_sectors = total_sectors - (bpb.reserved_sectors as u32 + fat_sectors + root_dir_sectors as u32);
         let total_clusters = data_sectors / bpb.sectors_per_cluster as u32;
+        let max_valid_cluster_number = total_clusters + RESERVED_FAT_ENTRIES;
         let fat_type = FatType::from_clusters(total_clusters);
 
         // read FSInfo sector if this is FAT32
@@ -501,6 +507,22 @@ impl<T: ReadWriteSeek> FileSystem<T> {
             fs_info.free_cluster_count = None;
         }
 
+        // Validate the numbers stored in the free_cluster_count and next_free_cluster are within bounds for volume
+        match fs_info.free_cluster_count {
+            Some(n) if n > total_clusters => {
+                warn!("invalid free_cluster_count ({}) in fs_info exceeds total cluster count ({})", n, total_clusters);
+                fs_info.free_cluster_count = None;
+            },
+            _ => {},
+        };
+        match fs_info.next_free_cluster {
+            Some(n) if n > max_valid_cluster_number => {
+                warn!("invalid free_cluster_count ({}) in fs_info exceeds maximum cluster number ({})", n, max_valid_cluster_number);
+                fs_info.next_free_cluster = None;
+            },
+            _ => {},
+        };
+
         // return FileSystem struct
         let status_flags = bpb.status_flags();
         Ok(FileSystem {

src/table.rs

@@ -1,5 +1,5 @@
 use io;
-
+use io::{Error, ErrorKind};
 use byteorder::LittleEndian;
 use byteorder_ext::{ReadBytesExt, WriteBytesExt};
 
@@ -14,7 +14,7 @@ type Fat12 = Fat<u8>;
 type Fat16 = Fat<u16>;
 type Fat32 = Fat<u32>;
 
-const RESERVED_FAT_ENTRIES: u32 = 2;
+pub const RESERVED_FAT_ENTRIES: u32 = 2;
 
 #[derive(Copy, Clone, Eq, PartialEq, Debug)]
 enum FatValue {
@@ -273,27 +273,49 @@ impl FatTrait for Fat16 {
 impl FatTrait for Fat32 {
     fn get_raw<T: ReadSeek>(fat: &mut T, cluster: u32) -> io::Result<u32> {
         fat.seek(io::SeekFrom::Start((cluster * 4) as u64))?;
-        Ok(fat.read_u32::<LittleEndian>()? & 0x0FFFFFFF)
+        Ok(fat.read_u32::<LittleEndian>()?)
     }
 
     fn get<T: ReadSeek>(fat: &mut T, cluster: u32) -> io::Result<FatValue> {
-        let val = Self::get_raw(fat, cluster)?;
+        let val = Self::get_raw(fat, cluster)? & 0x0FFFFFFF;
         Ok(match val {
+            0 if cluster >= 0x0FFFFFF7 && cluster <= 0x0FFFFFFF => {
+                let tmp = if cluster == 0x0FFFFFF7 { "BAD_CLUSTER" } else { "end-of-chain" };
+                warn!("cluster number {} is a special value in FAT to indicate {}; it should never be seen as free", cluster, tmp);
+                FatValue::Bad // avoid accidental use or allocation into a FAT chain
+            },
             0 => FatValue::Free,
             0x0FFFFFF7 => FatValue::Bad,
             0x0FFFFFF8...0x0FFFFFFF => FatValue::EndOfChain,
+            n if cluster >= 0x0FFFFFF7 && cluster <= 0x0FFFFFFF => {
+                let tmp = if cluster == 0x0FFFFFF7 { "BAD_CLUSTER" } else { "end-of-chain" };
+                warn!("cluster number {} is a special value in FAT to indicate {}; hiding potential FAT chain value {} and instead reporting as a bad sector", cluster, tmp, n);
+                FatValue::Bad // avoid accidental use or allocation into a FAT chain
+            },
             n => FatValue::Data(n as u32),
         })
     }
 
     fn set<T: ReadWriteSeek>(fat: &mut T, cluster: u32, value: FatValue) -> io::Result<()> {
+        let old_reserved_bits = Self::get_raw(fat, cluster)? & 0xF0000000;
         fat.seek(io::SeekFrom::Start((cluster * 4) as u64))?;
+
+        if value == FatValue::Free && cluster >= 0x0FFFFFF7 && cluster <= 0x0FFFFFFF {
+            // NOTE: it is technically allowed for them to store FAT chain loops,
+            //       or even have them all store value '4' as their next cluster.
+            //       Some believe only FatValue::Bad should be allowed for this edge case.
+            let tmp = if cluster == 0x0FFFFFF7 { "BAD_CLUSTER" } else { "end-of-chain" };
+            let msg = format!("cluster number {} is a special value in FAT to indicate {}; it should never be set as free", cluster, tmp);
+            let custom_error = Error::new(ErrorKind::Other, msg);
+            return Err(custom_error);
+        };
         let raw_val = match value {
             FatValue::Free => 0,
             FatValue::Bad => 0x0FFFFFF7,
             FatValue::EndOfChain => 0x0FFFFFFF,
             FatValue::Data(n) => n,
         };
+        let raw_val = raw_val | old_reserved_bits; // must preserve original reserved values
         fat.write_u32::<LittleEndian>(raw_val)?;
         Ok(())
     }