From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=66.187.233.73; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 847CB209605CE for ; Tue, 11 Sep 2018 09:02:08 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C4C2A406CBC0; Tue, 11 Sep 2018 16:02:07 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-36.rdu2.redhat.com [10.10.120.36]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1F3D820389E0; Tue, 11 Sep 2018 16:02:06 +0000 (UTC) To: "Robinson, Herbie" References: Cc: "edk2-devel@lists.01.org" , Ruiyu Ni From: Laszlo Ersek Message-ID: <3ec6170f-8aaa-4bcf-69ac-0664fed2204f@redhat.com> Date: Tue, 11 Sep 2018 18:02:06 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.78 on 10.11.54.6 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.6]); Tue, 11 Sep 2018 16:02:07 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.6]); Tue, 11 Sep 2018 16:02:07 +0000 (UTC) for IP:'10.11.54.6' DOMAIN:'int-mx06.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'lersek@redhat.com' RCPT:'' Subject: Re: [PATCH 1/1] FatPkg/EnhancedFatDxe Fix Double Cluster Allocation X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 11 Sep 2018 16:02:09 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 09/07/18 02:07, Robinson, Herbie wrote: > 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 CC'ing Ray (see "Maintainers.txt"). Thanks Laszlo > --- > 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 > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel >