Reviewed-by: Ruiyu Ni <[email protected]> Thanks/Ray
> -----Original Message----- > From: edk2-devel <[email protected]> On Behalf Of > Robinson, Herbie > Sent: Friday, September 7, 2018 8:07 AM > To: [email protected] > Subject: [edk2] [PATCH 1/1] FatPkg/EnhancedFatDxe Fix Double Cluster > Allocation > > This is a fix for a double cluster allocation when the disk is full. The > double > allocation happens because FatGrowEof calls FatAllocateCluster without > immediately marking the each returned cluster as allocated. The fix is to > move the FatSetFatEntry call inside the loop. I've also include some > improvements to the sanity checks that I added while tracking this down. > They are optional. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by:Herbie Robinson <[email protected]> > --- > diff --git a/FatPkg/EnhancedFatDxe/FileSpace.c > b/FatPkg/EnhancedFatDxe/FileSpace.c > index 1254cd6..e17d3b6 100644 > --- a/FatPkg/EnhancedFatDxe/FileSpace.c > +++ b/FatPkg/EnhancedFatDxe/FileSpace.c > @@ -467,7 +467,7 @@ FatGrowEof ( > ClusterCount = 0; > > while (!FAT_END_OF_FAT_CHAIN (Cluster)) { > - if (Cluster == FAT_CLUSTER_FREE || Cluster >= FAT_CLUSTER_SPECIAL) { > + if (Cluster < FAT_MIN_CLUSTER || Cluster > Volume->MaxCluster + 1) { > > DEBUG ( > (EFI_D_INIT | EFI_D_ERROR, > @@ -509,6 +509,11 @@ FatGrowEof ( > goto Done; > } > > + if (NewCluster < FAT_MIN_CLUSTER || NewCluster > Volume- > >MaxCluster + 1) { > + Status = EFI_VOLUME_CORRUPTED; > + goto Done; > + } > + > if (LastCluster != 0) { > FatSetFatEntry (Volume, LastCluster, NewCluster); > } else { > @@ -518,12 +523,21 @@ FatGrowEof ( > > LastCluster = NewCluster; > CurSize += 1; > + > + // > + // Terminate the cluster list > + // > + // Note that we must do this EVERY time we allocate a cluster, because > + // FatAllocateCluster scans the FAT looking for a free cluster and > + // "LastCluster" is no longer free! Usually, FatAllocateCluster will > + // start looking with the cluster after "LastCluster"; however, when > + // there is only one free cluster left, it will find "LastCluster" > + // a second time. There are other, less predictable scenarios > + // where this could happen, as well. > + // > + FatSetFatEntry (Volume, LastCluster, (UINTN) FAT_CLUSTER_LAST); > + OFile->FileLastCluster = LastCluster; > } > - // > - // Terminate the cluster list > - // > - FatSetFatEntry (Volume, LastCluster, (UINTN) FAT_CLUSTER_LAST); > - OFile->FileLastCluster = LastCluster; > } > > OFile->FileSize = (UINTN) NewSizeInBytes; > @@ -603,7 +617,7 @@ FatOFilePosition ( > Cluster = FatGetFatEntry (Volume, Cluster); > } > > - if (Cluster < FAT_MIN_CLUSTER) { > + if (Cluster < FAT_MIN_CLUSTER || Cluster > Volume->MaxCluster + 1) { > return EFI_VOLUME_CORRUPTED; > } > > _______________________________________________ > edk2-devel mailing list > [email protected] > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
