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 BA372207E6370 for ; Mon, 9 Jul 2018 17:24:51 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id EC1A3401CC41; Tue, 10 Jul 2018 00:24:50 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-121-21.rdu2.redhat.com [10.10.121.21]) by smtp.corp.redhat.com (Postfix) with ESMTP id 0EC7B2026D65; Tue, 10 Jul 2018 00:24:49 +0000 (UTC) From: Laszlo Ersek To: Roman Bacik , edk2-devel@lists.01.org Cc: Chao Zhang , Jiewen Yao , Vladimir Olovyannikov References: <4e3b21af-4aeb-1524-df25-a1d7e08fdc94@redhat.com> Message-ID: <9160437f-6dbd-ec35-71a4-f6aa2a305881@redhat.com> Date: Tue, 10 Jul 2018 02:24:49 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: <4e3b21af-4aeb-1524-df25-a1d7e08fdc94@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.5]); Tue, 10 Jul 2018 00:24:50 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.5]); Tue, 10 Jul 2018 00:24:50 +0000 (UTC) for IP:'10.11.54.4' DOMAIN:'int-mx04.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'lersek@redhat.com' RCPT:'' Subject: Re: [PATCH v1] SecurityPkg: Fix assert when setting key from FAT formatted eMMC/SD/USB X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 10 Jul 2018 00:24:51 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 07/10/18 02:02, Laszlo Ersek wrote: > On 07/10/18 00:11, Roman Bacik wrote: >> + PathName = AllocateZeroPool (PathLength); >> + CopyMem (PathName, ((FILEPATH_DEVICE_PATH*)*FilePath)->PathName, >> PathLength); > > (3) I think it's not necessary to zero-fill the buffer, we're going to > overwrite it right after the allocation. > > There's a convenience function for that: AllocateCopyPool(). > > (4) The number of bytes is not correct IMO. "PathLength" stands for the > number of bytes in the entire device path node (FILEPATH_DEVICE_PATH), > including Header and PathName. So, for getting the number of bytes in > just PathName, we should subtract the size of Header. > > Presently, we over-read the source buffer; it's not causing problems > because PathName is NUL-terminated. Sorry, I was unclear; I meant there were no *observable* problems. The over-read of the source buffer results in garbage at the end of the target buffer, which are later not consumed due to the terminating NUL appearing earlier. Still, we should not over-read the source buffer. Thanks Laszlo