[PATCH 0/2] UefiBootManagerLib, HttpDxe: tweaks for large HTTP(S) downloads

[PATCH 0/2] UefiBootManagerLib, HttpDxe: tweaks for large HTTP(S) downloads

[Laszlo Ersek]

Repo:   https://github.com/lersek/edk2.git
Branch: tweaks_for_large_http

This series aims to improve HTTP(S) Boot experience with large (4GiB+)
files.

Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>

Thanks,
Laszlo

Laszlo Ersek (2):
  MdeModulePkg/UefiBootManagerLib: log reserved mem allocation failure
  NetworkPkg/HttpDxe: fix 32-bit truncation in HTTPS download

 MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 31 ++++++++++++++++++++
 NetworkPkg/HttpDxe/HttpImpl.c                    |  2 +-
 2 files changed, 32 insertions(+), 1 deletion(-)

-- 
2.19.1.3.g30247aa5d201

[PATCH 1/2] MdeModulePkg/UefiBootManagerLib: log reserved mem allocation failure

[Laszlo Ersek]

The LoadFile protocol can report such a large buffer size that we cannot
allocate enough reserved pages for. This particularly affects HTTP(S)
Boot, if the remote file is very large (for example, an ISO image).

While the TianoCore wiki mentions this at
<https://github.com/tianocore/tianocore.github.io/wiki/HTTP-Boot#ram-disk-image-size>:

> The maximum RAM disk image size depends on how much continuous reserved
> memory block the platform could provide.

it's hard to remember; so log a DEBUG_ERROR message when the allocation
fails.

This patch produces error messages such as:

> UiApp:BmExpandLoadFile: failed to allocate reserved pages:
> BufferSize=4501536768
> LoadFile="PciRoot(0x0)/Pci(0x3,0x0)/MAC(5254001B103E,0x1)/
>      IPv4(0.0.0.0,TCP,DHCP,192.168.124.106,192.168.124.1,255.255.255.0)/
>      Dns(192.168.124.1)/
>      Uri(https://ipv4-server/RHEL-7.7-20190723.1-Server-x86_64-dvd1.iso)"
> FilePath=""

(Manually rewrapped here for keeping PatchCheck.py happy.)

Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 31 ++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index 62c5b2dc94ab..540d169ec1a6 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -1387,6 +1387,37 @@ BmExpandLoadFile (
   //
   FileBuffer = AllocateReservedPages (EFI_SIZE_TO_PAGES (BufferSize));
   if (FileBuffer == NULL) {
+    DEBUG_CODE (
+      EFI_DEVICE_PATH *LoadFilePath;
+      CHAR16          *LoadFileText;
+      CHAR16          *FileText;
+
+      LoadFilePath = DevicePathFromHandle (LoadFileHandle);
+      if (LoadFilePath == NULL) {
+        LoadFileText = NULL;
+      } else {
+        LoadFileText = ConvertDevicePathToText (LoadFilePath, FALSE, FALSE);
+      }
+      FileText = ConvertDevicePathToText (FilePath, FALSE, FALSE);
+
+      DEBUG ((
+        DEBUG_ERROR,
+        "%a:%a: failed to allocate reserved pages: "
+        "BufferSize=%Lu LoadFile=\"%s\" FilePath=\"%s\"\n",
+        gEfiCallerBaseName,
+        __FUNCTION__,
+        (UINT64)BufferSize,
+        LoadFileText,
+        FileText
+        ));
+
+      if (FileText != NULL) {
+        FreePool (FileText);
+      }
+      if (LoadFileText != NULL) {
+        FreePool (LoadFileText);
+      }
+      );
     return NULL;
   }
 
-- 
2.19.1.3.g30247aa5d201

[PATCH 2/2] NetworkPkg/HttpDxe: fix 32-bit truncation in HTTPS download

[Laszlo Ersek]

When downloading over TLS, each TLS message ("APP packet") is returned as
a (decrypted) fragment table by EFI_TLS_PROTOCOL.ProcessPacket().

The TlsProcessMessage() function in "NetworkPkg/HttpDxe/HttpsSupport.c"
linearizes the fragment table into a single contiguous data block. The
resultant flat data block contains both TLS headers and data.

The HttpsReceive() function parses the actual application data -- in this
case: decrypted HTTP data -- out of the flattened TLS data block, peeling
off the TLS headers.

The HttpResponseWorker() function in "NetworkPkg/HttpDxe/HttpImpl.c"
propagates this HTTP data outwards, implementing the
EFI_HTTP_PROTOCOL.Response() function.

Now consider the following documentation for EFI_HTTP_PROTOCOL.Response(),
quoted from "MdePkg/Include/Protocol/Http.h":

> It is the responsibility of the caller to allocate a buffer for Body and
> specify the size in BodyLength. If the remote host provides a response
> that contains a content body, up to BodyLength bytes will be copied from
> the receive buffer into Body and BodyLength will be updated with the
> amount of bytes received and copied to Body. This allows the client to
> download a large file in chunks instead of into one contiguous block of
> memory.

Note that, if the caller-allocated buffer is larger than the
server-provided chunk, then the transfer length is limited by the latter.
This is in fact the dominant case when downloading a huge file (for which
UefiBootManagerLib allocated a huge contiguous RAM Disk buffer) in small
TLS messages.

For adjusting BodyLength as described above -- i.e., to the application
data chunk that has been extracted from the TLS message --, the
HttpResponseWorker() function employs the following assignment:

    HttpMsg->BodyLength = MIN (Fragment.Len, (UINT32) HttpMsg->BodyLength);

The (UINT32) cast is motivated by the MIN() requirement -- in
"MdePkg/Include/Base.h" -- that both arguments be of the same type.

"Fragment.Len" (NET_FRAGMENT.Len) has type UINT32, and
"HttpMsg->BodyLength" (EFI_HTTP_MESSAGE.BodyLength) has type UINTN.
Therefore a cast is indeed necessary.

Unfortunately, the cast is done in the wrong direction. Consider the
following circumstances:

- "Fragment.Len" happens to be consistently 16KiB, dictated by the HTTPS
  Server's TLS stack,

- the size of the file to download is 4GiB + N*16KiB, where N is a
  positive integer.

As the download progresses, each received 16KiB application data chunk
brings the *next* input value of BodyLength closer down to 4GiB. The cast
in MIN() always masks off the high-order bits from the input value of
BodyLength, but this is no problem because the low-order bits are nonzero,
therefore the MIN() always permits progress.

However, once BodyLength reaches 4GiB exactly on input, the MIN()
invocation produces a zero value. HttpResponseWorker() adjusts the output
value of BodyLength to zero, and then passes it to HttpParseMessageBody().

HttpParseMessageBody() (in "NetworkPkg/Library/DxeHttpLib/DxeHttpLib.c")
rejects the zero BodyLength with EFI_INVALID_PARAMETER, which is fully
propagated outwards, and aborts the HTTPS download. HttpBootDxe writes the
message "Error: Unexpected network error" to the UEFI console.

For example, a file with size (4GiB + 197MiB) terminates after downloading
just 197MiB.

Invert the direction of the cast: widen "Fragment.Len" to UINTN.

Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 NetworkPkg/HttpDxe/HttpImpl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/NetworkPkg/HttpDxe/HttpImpl.c b/NetworkPkg/HttpDxe/HttpImpl.c
index 6b877314bd57..1acbb60d1014 100644
--- a/NetworkPkg/HttpDxe/HttpImpl.c
+++ b/NetworkPkg/HttpDxe/HttpImpl.c
@@ -1348,7 +1348,7 @@ HttpResponseWorker (
     //
     // Process the received the body packet.
     //
-    HttpMsg->BodyLength = MIN (Fragment.Len, (UINT32) HttpMsg->BodyLength);
+    HttpMsg->BodyLength = MIN ((UINTN) Fragment.Len, HttpMsg->BodyLength);
 
     CopyMem (HttpMsg->Body, Fragment.Bulk, HttpMsg->BodyLength);
 
-- 
2.19.1.3.g30247aa5d201

Re: [PATCH 1/2] MdeModulePkg/UefiBootManagerLib: log reserved mem allocation failure

[Ni, Ray]

+ Siyuan who originally worked in the HTTP boot functionality.

> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Thursday, January 9, 2020 7:43 AM
> To: edk2-devel-groups-io <devel@edk2.groups.io>
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
> Ni, Ray <ray.ni@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>
> Subject: [PATCH 1/2] MdeModulePkg/UefiBootManagerLib: log reserved
> mem allocation failure
> 
> The LoadFile protocol can report such a large buffer size that we cannot
> allocate enough reserved pages for. This particularly affects HTTP(S)
> Boot, if the remote file is very large (for example, an ISO image).
> 
> While the TianoCore wiki mentions this at
> <https://github.com/tianocore/tianocore.github.io/wiki/HTTP-Boot#ram-
> disk-image-size>:
> 
> > The maximum RAM disk image size depends on how much continuous
> reserved
> > memory block the platform could provide.
> 
> it's hard to remember; so log a DEBUG_ERROR message when the allocation
> fails.
> 
> This patch produces error messages such as:
> 
> > UiApp:BmExpandLoadFile: failed to allocate reserved pages:
> > BufferSize=4501536768
> > LoadFile="PciRoot(0x0)/Pci(0x3,0x0)/MAC(5254001B103E,0x1)/
> >      IPv4(0.0.0.0,TCP,DHCP,192.168.124.106,192.168.124.1,255.255.255.0)/
> >      Dns(192.168.124.1)/
> >      Uri(https://ipv4-server/RHEL-7.7-20190723.1-Server-x86_64-dvd1.iso)"
> > FilePath=""
> 
> (Manually rewrapped here for keeping PatchCheck.py happy.)
> 
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Zhichao Gao <zhichao.gao@intel.com>
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 31
> ++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> index 62c5b2dc94ab..540d169ec1a6 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> @@ -1387,6 +1387,37 @@ BmExpandLoadFile (
>    //
>    FileBuffer = AllocateReservedPages (EFI_SIZE_TO_PAGES (BufferSize));
>    if (FileBuffer == NULL) {
> +    DEBUG_CODE (
> +      EFI_DEVICE_PATH *LoadFilePath;
> +      CHAR16          *LoadFileText;
> +      CHAR16          *FileText;
> +
> +      LoadFilePath = DevicePathFromHandle (LoadFileHandle);
> +      if (LoadFilePath == NULL) {
> +        LoadFileText = NULL;
> +      } else {
> +        LoadFileText = ConvertDevicePathToText (LoadFilePath, FALSE, FALSE);
> +      }
> +      FileText = ConvertDevicePathToText (FilePath, FALSE, FALSE);
> +
> +      DEBUG ((
> +        DEBUG_ERROR,
> +        "%a:%a: failed to allocate reserved pages: "
> +        "BufferSize=%Lu LoadFile=\"%s\" FilePath=\"%s\"\n",
> +        gEfiCallerBaseName,
> +        __FUNCTION__,
> +        (UINT64)BufferSize,
> +        LoadFileText,
> +        FileText
> +        ));
> +
> +      if (FileText != NULL) {
> +        FreePool (FileText);
> +      }
> +      if (LoadFileText != NULL) {
> +        FreePool (LoadFileText);
> +      }
> +      );
>      return NULL;
>    }
> 
> --
> 2.19.1.3.g30247aa5d201
> 

Re: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix 32-bit truncation in HTTPS download

[Philippe Mathieu-Daudé]

On 1/9/20 12:43 AM, Laszlo Ersek wrote:
> When downloading over TLS, each TLS message ("APP packet") is returned as
> a (decrypted) fragment table by EFI_TLS_PROTOCOL.ProcessPacket().
> 
> The TlsProcessMessage() function in "NetworkPkg/HttpDxe/HttpsSupport.c"
> linearizes the fragment table into a single contiguous data block. The
> resultant flat data block contains both TLS headers and data.
> 
> The HttpsReceive() function parses the actual application data -- in this
> case: decrypted HTTP data -- out of the flattened TLS data block, peeling
> off the TLS headers.
> 
> The HttpResponseWorker() function in "NetworkPkg/HttpDxe/HttpImpl.c"
> propagates this HTTP data outwards, implementing the
> EFI_HTTP_PROTOCOL.Response() function.
> 
> Now consider the following documentation for EFI_HTTP_PROTOCOL.Response(),
> quoted from "MdePkg/Include/Protocol/Http.h":
> 
>> It is the responsibility of the caller to allocate a buffer for Body and
>> specify the size in BodyLength. If the remote host provides a response
>> that contains a content body, up to BodyLength bytes will be copied from
>> the receive buffer into Body and BodyLength will be updated with the
>> amount of bytes received and copied to Body. This allows the client to
>> download a large file in chunks instead of into one contiguous block of
>> memory.
> 
> Note that, if the caller-allocated buffer is larger than the
> server-provided chunk, then the transfer length is limited by the latter.
> This is in fact the dominant case when downloading a huge file (for which
> UefiBootManagerLib allocated a huge contiguous RAM Disk buffer) in small
> TLS messages.
> 
> For adjusting BodyLength as described above -- i.e., to the application
> data chunk that has been extracted from the TLS message --, the
> HttpResponseWorker() function employs the following assignment:
> 
>      HttpMsg->BodyLength = MIN (Fragment.Len, (UINT32) HttpMsg->BodyLength);
> 
> The (UINT32) cast is motivated by the MIN() requirement -- in
> "MdePkg/Include/Base.h" -- that both arguments be of the same type.
> 
> "Fragment.Len" (NET_FRAGMENT.Len) has type UINT32, and
> "HttpMsg->BodyLength" (EFI_HTTP_MESSAGE.BodyLength) has type UINTN.
> Therefore a cast is indeed necessary.
> 
> Unfortunately, the cast is done in the wrong direction. Consider the
> following circumstances:
> 
> - "Fragment.Len" happens to be consistently 16KiB, dictated by the HTTPS
>    Server's TLS stack,
> 
> - the size of the file to download is 4GiB + N*16KiB, where N is a
>    positive integer.
> 
> As the download progresses, each received 16KiB application data chunk
> brings the *next* input value of BodyLength closer down to 4GiB. The cast
> in MIN() always masks off the high-order bits from the input value of
> BodyLength, but this is no problem because the low-order bits are nonzero,
> therefore the MIN() always permits progress.
> 
> However, once BodyLength reaches 4GiB exactly on input, the MIN()
> invocation produces a zero value. HttpResponseWorker() adjusts the output
> value of BodyLength to zero, and then passes it to HttpParseMessageBody().
> 
> HttpParseMessageBody() (in "NetworkPkg/Library/DxeHttpLib/DxeHttpLib.c")
> rejects the zero BodyLength with EFI_INVALID_PARAMETER, which is fully
> propagated outwards, and aborts the HTTPS download. HttpBootDxe writes the
> message "Error: Unexpected network error" to the UEFI console.

Thanks for the very detailed explanation.

Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>

> 
> For example, a file with size (4GiB + 197MiB) terminates after downloading
> just 197MiB.
> 
> Invert the direction of the cast: widen "Fragment.Len" to UINTN.
> 
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
> Cc: Siyuan Fu <siyuan.fu@intel.com>
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>   NetworkPkg/HttpDxe/HttpImpl.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/NetworkPkg/HttpDxe/HttpImpl.c b/NetworkPkg/HttpDxe/HttpImpl.c
> index 6b877314bd57..1acbb60d1014 100644
> --- a/NetworkPkg/HttpDxe/HttpImpl.c
> +++ b/NetworkPkg/HttpDxe/HttpImpl.c
> @@ -1348,7 +1348,7 @@ HttpResponseWorker (
>       //
>       // Process the received the body packet.
>       //
> -    HttpMsg->BodyLength = MIN (Fragment.Len, (UINT32) HttpMsg->BodyLength);
> +    HttpMsg->BodyLength = MIN ((UINTN) Fragment.Len, HttpMsg->BodyLength);
>   
>       CopyMem (HttpMsg->Body, Fragment.Bulk, HttpMsg->BodyLength);
>   
> 

Re: [edk2-devel] [PATCH 1/2] MdeModulePkg/UefiBootManagerLib: log reserved mem allocation failure

[Philippe Mathieu-Daudé]

On 1/9/20 12:43 AM, Laszlo Ersek wrote:
> The LoadFile protocol can report such a large buffer size that we cannot
> allocate enough reserved pages for. This particularly affects HTTP(S)
> Boot, if the remote file is very large (for example, an ISO image).
> 
> While the TianoCore wiki mentions this at
> <https://github.com/tianocore/tianocore.github.io/wiki/HTTP-Boot#ram-disk-image-size>:
> 
>> The maximum RAM disk image size depends on how much continuous reserved
>> memory block the platform could provide.
> 
> it's hard to remember; so log a DEBUG_ERROR message when the allocation
> fails.
> 
> This patch produces error messages such as:
> 
>> UiApp:BmExpandLoadFile: failed to allocate reserved pages:
>> BufferSize=4501536768
>> LoadFile="PciRoot(0x0)/Pci(0x3,0x0)/MAC(5254001B103E,0x1)/
>>       IPv4(0.0.0.0,TCP,DHCP,192.168.124.106,192.168.124.1,255.255.255.0)/
>>       Dns(192.168.124.1)/
>>       Uri(https://ipv4-server/RHEL-7.7-20190723.1-Server-x86_64-dvd1.iso)"
>> FilePath=""
> 
> (Manually rewrapped here for keeping PatchCheck.py happy.)
> 
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Zhichao Gao <zhichao.gao@intel.com>
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>   MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 31 ++++++++++++++++++++
>   1 file changed, 31 insertions(+)
> 
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> index 62c5b2dc94ab..540d169ec1a6 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> @@ -1387,6 +1387,37 @@ BmExpandLoadFile (
>     //
>     FileBuffer = AllocateReservedPages (EFI_SIZE_TO_PAGES (BufferSize));
>     if (FileBuffer == NULL) {
> +    DEBUG_CODE (
> +      EFI_DEVICE_PATH *LoadFilePath;
> +      CHAR16          *LoadFileText;
> +      CHAR16          *FileText;
> +
> +      LoadFilePath = DevicePathFromHandle (LoadFileHandle);
> +      if (LoadFilePath == NULL) {
> +        LoadFileText = NULL;
> +      } else {
> +        LoadFileText = ConvertDevicePathToText (LoadFilePath, FALSE, FALSE);
> +      }
> +      FileText = ConvertDevicePathToText (FilePath, FALSE, FALSE);
> +
> +      DEBUG ((
> +        DEBUG_ERROR,
> +        "%a:%a: failed to allocate reserved pages: "
> +        "BufferSize=%Lu LoadFile=\"%s\" FilePath=\"%s\"\n",
> +        gEfiCallerBaseName,
> +        __FUNCTION__,
> +        (UINT64)BufferSize,
> +        LoadFileText,
> +        FileText
> +        ));
> +
> +      if (FileText != NULL) {
> +        FreePool (FileText);

Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>

> +      }
> +      if (LoadFileText != NULL) {
> +        FreePool (LoadFileText);
> +      }
> +      );
>       return NULL;
>     }
>   
> 

Re: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix 32-bit truncation in HTTPS download

[Siyuan, Fu]

Reviewed-by: Siyuan Fu <siyuan.fu@intel.com>

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo
> Ersek
> Sent: 2020年1月9日 7:43
> To: edk2-devel-groups-io <devel@edk2.groups.io>
> Cc: Wu, Jiaxin <jiaxin.wu@intel.com>; Maciej Rabeda
> <maciej.rabeda@linux.intel.com>; Fu, Siyuan <siyuan.fu@intel.com>
> Subject: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix 32-bit truncation
> in HTTPS download
> 
> When downloading over TLS, each TLS message ("APP packet") is returned as
> a (decrypted) fragment table by EFI_TLS_PROTOCOL.ProcessPacket().
> 
> The TlsProcessMessage() function in "NetworkPkg/HttpDxe/HttpsSupport.c"
> linearizes the fragment table into a single contiguous data block. The
> resultant flat data block contains both TLS headers and data.
> 
> The HttpsReceive() function parses the actual application data -- in this
> case: decrypted HTTP data -- out of the flattened TLS data block, peeling
> off the TLS headers.
> 
> The HttpResponseWorker() function in "NetworkPkg/HttpDxe/HttpImpl.c"
> propagates this HTTP data outwards, implementing the
> EFI_HTTP_PROTOCOL.Response() function.
> 
> Now consider the following documentation for
> EFI_HTTP_PROTOCOL.Response(),
> quoted from "MdePkg/Include/Protocol/Http.h":
> 
> > It is the responsibility of the caller to allocate a buffer for Body and
> > specify the size in BodyLength. If the remote host provides a response
> > that contains a content body, up to BodyLength bytes will be copied from
> > the receive buffer into Body and BodyLength will be updated with the
> > amount of bytes received and copied to Body. This allows the client to
> > download a large file in chunks instead of into one contiguous block of
> > memory.
> 
> Note that, if the caller-allocated buffer is larger than the
> server-provided chunk, then the transfer length is limited by the latter.
> This is in fact the dominant case when downloading a huge file (for which
> UefiBootManagerLib allocated a huge contiguous RAM Disk buffer) in small
> TLS messages.
> 
> For adjusting BodyLength as described above -- i.e., to the application
> data chunk that has been extracted from the TLS message --, the
> HttpResponseWorker() function employs the following assignment:
> 
>     HttpMsg->BodyLength = MIN (Fragment.Len, (UINT32) HttpMsg-
> >BodyLength);
> 
> The (UINT32) cast is motivated by the MIN() requirement -- in
> "MdePkg/Include/Base.h" -- that both arguments be of the same type.
> 
> "Fragment.Len" (NET_FRAGMENT.Len) has type UINT32, and
> "HttpMsg->BodyLength" (EFI_HTTP_MESSAGE.BodyLength) has type UINTN.
> Therefore a cast is indeed necessary.
> 
> Unfortunately, the cast is done in the wrong direction. Consider the
> following circumstances:
> 
> - "Fragment.Len" happens to be consistently 16KiB, dictated by the HTTPS
>   Server's TLS stack,
> 
> - the size of the file to download is 4GiB + N*16KiB, where N is a
>   positive integer.
> 
> As the download progresses, each received 16KiB application data chunk
> brings the *next* input value of BodyLength closer down to 4GiB. The cast
> in MIN() always masks off the high-order bits from the input value of
> BodyLength, but this is no problem because the low-order bits are nonzero,
> therefore the MIN() always permits progress.
> 
> However, once BodyLength reaches 4GiB exactly on input, the MIN()
> invocation produces a zero value. HttpResponseWorker() adjusts the output
> value of BodyLength to zero, and then passes it to HttpParseMessageBody().
> 
> HttpParseMessageBody() (in "NetworkPkg/Library/DxeHttpLib/DxeHttpLib.c")
> rejects the zero BodyLength with EFI_INVALID_PARAMETER, which is fully
> propagated outwards, and aborts the HTTPS download. HttpBootDxe writes
> the
> message "Error: Unexpected network error" to the UEFI console.
> 
> For example, a file with size (4GiB + 197MiB) terminates after downloading
> just 197MiB.
> 
> Invert the direction of the cast: widen "Fragment.Len" to UINTN.
> 
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
> Cc: Siyuan Fu <siyuan.fu@intel.com>
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  NetworkPkg/HttpDxe/HttpImpl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/NetworkPkg/HttpDxe/HttpImpl.c
> b/NetworkPkg/HttpDxe/HttpImpl.c
> index 6b877314bd57..1acbb60d1014 100644
> --- a/NetworkPkg/HttpDxe/HttpImpl.c
> +++ b/NetworkPkg/HttpDxe/HttpImpl.c
> @@ -1348,7 +1348,7 @@ HttpResponseWorker (
>      //
>      // Process the received the body packet.
>      //
> -    HttpMsg->BodyLength = MIN (Fragment.Len, (UINT32) HttpMsg-
> >BodyLength);
> +    HttpMsg->BodyLength = MIN ((UINTN) Fragment.Len, HttpMsg-
> >BodyLength);
> 
>      CopyMem (HttpMsg->Body, Fragment.Bulk, HttpMsg->BodyLength);
> 
> --
> 2.19.1.3.g30247aa5d201
> 
> 
> 

Re: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix 32-bit truncation in HTTPS download

[Laszlo Ersek]

On 01/10/20 02:01, Siyuan, Fu wrote:
> Reviewed-by: Siyuan Fu <siyuan.fu@intel.com>

Thanks, Siyuan! Can you please review patch#1 too in this series? (See
Ray's request: <https://edk2.groups.io/g/devel/message/53044>.)

Thanks!
Laszlo

Re: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix 32-bit truncation in HTTPS download

[Maciej Rabeda]

Thanks for a very thorough explanation!

Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>

On 09-Jan-20 00:43, Laszlo Ersek wrote:
> When downloading over TLS, each TLS message ("APP packet") is returned as
> a (decrypted) fragment table by EFI_TLS_PROTOCOL.ProcessPacket().
>
> The TlsProcessMessage() function in "NetworkPkg/HttpDxe/HttpsSupport.c"
> linearizes the fragment table into a single contiguous data block. The
> resultant flat data block contains both TLS headers and data.
>
> The HttpsReceive() function parses the actual application data -- in this
> case: decrypted HTTP data -- out of the flattened TLS data block, peeling
> off the TLS headers.
>
> The HttpResponseWorker() function in "NetworkPkg/HttpDxe/HttpImpl.c"
> propagates this HTTP data outwards, implementing the
> EFI_HTTP_PROTOCOL.Response() function.
>
> Now consider the following documentation for EFI_HTTP_PROTOCOL.Response(),
> quoted from "MdePkg/Include/Protocol/Http.h":
>
>> It is the responsibility of the caller to allocate a buffer for Body and
>> specify the size in BodyLength. If the remote host provides a response
>> that contains a content body, up to BodyLength bytes will be copied from
>> the receive buffer into Body and BodyLength will be updated with the
>> amount of bytes received and copied to Body. This allows the client to
>> download a large file in chunks instead of into one contiguous block of
>> memory.
> Note that, if the caller-allocated buffer is larger than the
> server-provided chunk, then the transfer length is limited by the latter.
> This is in fact the dominant case when downloading a huge file (for which
> UefiBootManagerLib allocated a huge contiguous RAM Disk buffer) in small
> TLS messages.
>
> For adjusting BodyLength as described above -- i.e., to the application
> data chunk that has been extracted from the TLS message --, the
> HttpResponseWorker() function employs the following assignment:
>
>      HttpMsg->BodyLength = MIN (Fragment.Len, (UINT32) HttpMsg->BodyLength);
>
> The (UINT32) cast is motivated by the MIN() requirement -- in
> "MdePkg/Include/Base.h" -- that both arguments be of the same type.
>
> "Fragment.Len" (NET_FRAGMENT.Len) has type UINT32, and
> "HttpMsg->BodyLength" (EFI_HTTP_MESSAGE.BodyLength) has type UINTN.
> Therefore a cast is indeed necessary.
>
> Unfortunately, the cast is done in the wrong direction. Consider the
> following circumstances:
>
> - "Fragment.Len" happens to be consistently 16KiB, dictated by the HTTPS
>    Server's TLS stack,
>
> - the size of the file to download is 4GiB + N*16KiB, where N is a
>    positive integer.
>
> As the download progresses, each received 16KiB application data chunk
> brings the *next* input value of BodyLength closer down to 4GiB. The cast
> in MIN() always masks off the high-order bits from the input value of
> BodyLength, but this is no problem because the low-order bits are nonzero,
> therefore the MIN() always permits progress.
>
> However, once BodyLength reaches 4GiB exactly on input, the MIN()
> invocation produces a zero value. HttpResponseWorker() adjusts the output
> value of BodyLength to zero, and then passes it to HttpParseMessageBody().
>
> HttpParseMessageBody() (in "NetworkPkg/Library/DxeHttpLib/DxeHttpLib.c")
> rejects the zero BodyLength with EFI_INVALID_PARAMETER, which is fully
> propagated outwards, and aborts the HTTPS download. HttpBootDxe writes the
> message "Error: Unexpected network error" to the UEFI console.
>
> For example, a file with size (4GiB + 197MiB) terminates after downloading
> just 197MiB.
>
> Invert the direction of the cast: widen "Fragment.Len" to UINTN.
>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
> Cc: Siyuan Fu <siyuan.fu@intel.com>
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>   NetworkPkg/HttpDxe/HttpImpl.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/NetworkPkg/HttpDxe/HttpImpl.c b/NetworkPkg/HttpDxe/HttpImpl.c
> index 6b877314bd57..1acbb60d1014 100644
> --- a/NetworkPkg/HttpDxe/HttpImpl.c
> +++ b/NetworkPkg/HttpDxe/HttpImpl.c
> @@ -1348,7 +1348,7 @@ HttpResponseWorker (
>       //
>       // Process the received the body packet.
>       //
> -    HttpMsg->BodyLength = MIN (Fragment.Len, (UINT32) HttpMsg->BodyLength);
> +    HttpMsg->BodyLength = MIN ((UINTN) Fragment.Len, HttpMsg->BodyLength);
>   
>       CopyMem (HttpMsg->Body, Fragment.Bulk, HttpMsg->BodyLength);
>   

Re: [edk2-devel] [PATCH 1/2] MdeModulePkg/UefiBootManagerLib: log reserved mem allocation failure

[Siyuan, Fu]

The change looks good to me.

Reviewed-by: Siyuan Fu <siyuan.fu@intel.com>

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo
> Ersek
> Sent: 2020年1月9日 7:43
> To: edk2-devel-groups-io <devel@edk2.groups.io>
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
> Ni, Ray <ray.ni@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>
> Subject: [edk2-devel] [PATCH 1/2] MdeModulePkg/UefiBootManagerLib: log
> reserved mem allocation failure
> 
> The LoadFile protocol can report such a large buffer size that we cannot
> allocate enough reserved pages for. This particularly affects HTTP(S)
> Boot, if the remote file is very large (for example, an ISO image).
> 
> While the TianoCore wiki mentions this at
> <https://github.com/tianocore/tianocore.github.io/wiki/HTTP-Boot#ram-
> disk-image-size>:
> 
> > The maximum RAM disk image size depends on how much continuous
> reserved
> > memory block the platform could provide.
> 
> it's hard to remember; so log a DEBUG_ERROR message when the allocation
> fails.
> 
> This patch produces error messages such as:
> 
> > UiApp:BmExpandLoadFile: failed to allocate reserved pages:
> > BufferSize=4501536768
> > LoadFile="PciRoot(0x0)/Pci(0x3,0x0)/MAC(5254001B103E,0x1)/
> >      IPv4(0.0.0.0,TCP,DHCP,192.168.124.106,192.168.124.1,255.255.255.0)/
> >      Dns(192.168.124.1)/
> >      Uri(https://ipv4-server/RHEL-7.7-20190723.1-Server-x86_64-dvd1.iso)"
> > FilePath=""
> 
> (Manually rewrapped here for keeping PatchCheck.py happy.)
> 
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Zhichao Gao <zhichao.gao@intel.com>
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 31
> ++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> index 62c5b2dc94ab..540d169ec1a6 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> @@ -1387,6 +1387,37 @@ BmExpandLoadFile (
>    //
>    FileBuffer = AllocateReservedPages (EFI_SIZE_TO_PAGES (BufferSize));
>    if (FileBuffer == NULL) {
> +    DEBUG_CODE (
> +      EFI_DEVICE_PATH *LoadFilePath;
> +      CHAR16          *LoadFileText;
> +      CHAR16          *FileText;
> +
> +      LoadFilePath = DevicePathFromHandle (LoadFileHandle);
> +      if (LoadFilePath == NULL) {
> +        LoadFileText = NULL;
> +      } else {
> +        LoadFileText = ConvertDevicePathToText (LoadFilePath, FALSE, FALSE);
> +      }
> +      FileText = ConvertDevicePathToText (FilePath, FALSE, FALSE);
> +
> +      DEBUG ((
> +        DEBUG_ERROR,
> +        "%a:%a: failed to allocate reserved pages: "
> +        "BufferSize=%Lu LoadFile=\"%s\" FilePath=\"%s\"\n",
> +        gEfiCallerBaseName,
> +        __FUNCTION__,
> +        (UINT64)BufferSize,
> +        LoadFileText,
> +        FileText
> +        ));
> +
> +      if (FileText != NULL) {
> +        FreePool (FileText);
> +      }
> +      if (LoadFileText != NULL) {
> +        FreePool (LoadFileText);
> +      }
> +      );
>      return NULL;
>    }
> 
> --
> 2.19.1.3.g30247aa5d201
> 
> 
> 
> 

Re: [edk2-devel] [PATCH 1/2] MdeModulePkg/UefiBootManagerLib: log reserved mem allocation failure

[Laszlo Ersek]

On 01/13/20 02:53, Siyuan, Fu wrote:
> The change looks good to me.
> 
> Reviewed-by: Siyuan Fu <siyuan.fu@intel.com>

Thank you, Siyuan!

Hao, Jian, can one of you please give a formal Acked-by, based on
Siyuan's and Phil's "Reviewed-by"s? (I'm asking because I'm not sure if
you would be comfortable with me pushing this without MdeModulePkg "M"
approval.)

Thanks!
Laszlo

>> -----Original Message-----
>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo
>> Ersek
>> Sent: 2020年1月9日 7:43
>> To: edk2-devel-groups-io <devel@edk2.groups.io>
>> Cc: Wu, Hao A <hao.a.wu@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
>> Ni, Ray <ray.ni@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>
>> Subject: [edk2-devel] [PATCH 1/2] MdeModulePkg/UefiBootManagerLib: log
>> reserved mem allocation failure
>>
>> The LoadFile protocol can report such a large buffer size that we cannot
>> allocate enough reserved pages for. This particularly affects HTTP(S)
>> Boot, if the remote file is very large (for example, an ISO image).
>>
>> While the TianoCore wiki mentions this at
>> <https://github.com/tianocore/tianocore.github.io/wiki/HTTP-Boot#ram-
>> disk-image-size>:
>>
>>> The maximum RAM disk image size depends on how much continuous
>> reserved
>>> memory block the platform could provide.
>>
>> it's hard to remember; so log a DEBUG_ERROR message when the allocation
>> fails.
>>
>> This patch produces error messages such as:
>>
>>> UiApp:BmExpandLoadFile: failed to allocate reserved pages:
>>> BufferSize=4501536768
>>> LoadFile="PciRoot(0x0)/Pci(0x3,0x0)/MAC(5254001B103E,0x1)/
>>>      IPv4(0.0.0.0,TCP,DHCP,192.168.124.106,192.168.124.1,255.255.255.0)/
>>>      Dns(192.168.124.1)/
>>>      Uri(https://ipv4-server/RHEL-7.7-20190723.1-Server-x86_64-dvd1.iso)"
>>> FilePath=""
>>
>> (Manually rewrapped here for keeping PatchCheck.py happy.)
>>
>> Cc: Hao A Wu <hao.a.wu@intel.com>
>> Cc: Jian J Wang <jian.j.wang@intel.com>
>> Cc: Ray Ni <ray.ni@intel.com>
>> Cc: Zhichao Gao <zhichao.gao@intel.com>
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>  MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 31
>> ++++++++++++++++++++
>>  1 file changed, 31 insertions(+)
>>
>> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
>> b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
>> index 62c5b2dc94ab..540d169ec1a6 100644
>> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
>> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
>> @@ -1387,6 +1387,37 @@ BmExpandLoadFile (
>>    //
>>    FileBuffer = AllocateReservedPages (EFI_SIZE_TO_PAGES (BufferSize));
>>    if (FileBuffer == NULL) {
>> +    DEBUG_CODE (
>> +      EFI_DEVICE_PATH *LoadFilePath;
>> +      CHAR16          *LoadFileText;
>> +      CHAR16          *FileText;
>> +
>> +      LoadFilePath = DevicePathFromHandle (LoadFileHandle);
>> +      if (LoadFilePath == NULL) {
>> +        LoadFileText = NULL;
>> +      } else {
>> +        LoadFileText = ConvertDevicePathToText (LoadFilePath, FALSE, FALSE);
>> +      }
>> +      FileText = ConvertDevicePathToText (FilePath, FALSE, FALSE);
>> +
>> +      DEBUG ((
>> +        DEBUG_ERROR,
>> +        "%a:%a: failed to allocate reserved pages: "
>> +        "BufferSize=%Lu LoadFile=\"%s\" FilePath=\"%s\"\n",
>> +        gEfiCallerBaseName,
>> +        __FUNCTION__,
>> +        (UINT64)BufferSize,
>> +        LoadFileText,
>> +        FileText
>> +        ));
>> +
>> +      if (FileText != NULL) {
>> +        FreePool (FileText);
>> +      }
>> +      if (LoadFileText != NULL) {
>> +        FreePool (LoadFileText);
>> +      }
>> +      );
>>      return NULL;
>>    }
>>
>> --
>> 2.19.1.3.g30247aa5d201
>>
>>
>>
>>
> 
> 
> 
> 

Re: [edk2-devel] [PATCH 1/2] MdeModulePkg/UefiBootManagerLib: log reserved mem allocation failure

[Wu, Hao A]

> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Laszlo Ersek
> Sent: Thursday, January 09, 2020 7:43 AM
> To: edk2-devel-groups-io
> Cc: Wu, Hao A; Wang, Jian J; Ni, Ray; Gao, Zhichao
> Subject: [edk2-devel] [PATCH 1/2] MdeModulePkg/UefiBootManagerLib: log
> reserved mem allocation failure
> 
> The LoadFile protocol can report such a large buffer size that we cannot
> allocate enough reserved pages for. This particularly affects HTTP(S)
> Boot, if the remote file is very large (for example, an ISO image).
> 
> While the TianoCore wiki mentions this at
> <https://github.com/tianocore/tianocore.github.io/wiki/HTTP-Boot#ram-
> disk-image-size>:
> 
> > The maximum RAM disk image size depends on how much continuous
> reserved
> > memory block the platform could provide.
> 
> it's hard to remember; so log a DEBUG_ERROR message when the allocation
> fails.
> 
> This patch produces error messages such as:
> 
> > UiApp:BmExpandLoadFile: failed to allocate reserved pages:
> > BufferSize=4501536768
> > LoadFile="PciRoot(0x0)/Pci(0x3,0x0)/MAC(5254001B103E,0x1)/
> >      IPv4(0.0.0.0,TCP,DHCP,192.168.124.106,192.168.124.1,255.255.255.0)/
> >      Dns(192.168.124.1)/
> >      Uri(https://ipv4-server/RHEL-7.7-20190723.1-Server-x86_64-dvd1.iso)"
> > FilePath=""


Acked-by: Hao A Wu <hao.a.wu@intel.com>

Best Regards,
Hao Wu


> 
> (Manually rewrapped here for keeping PatchCheck.py happy.)
> 
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Zhichao Gao <zhichao.gao@intel.com>
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 31
> ++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> index 62c5b2dc94ab..540d169ec1a6 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> @@ -1387,6 +1387,37 @@ BmExpandLoadFile (
>    //
>    FileBuffer = AllocateReservedPages (EFI_SIZE_TO_PAGES (BufferSize));
>    if (FileBuffer == NULL) {
> +    DEBUG_CODE (
> +      EFI_DEVICE_PATH *LoadFilePath;
> +      CHAR16          *LoadFileText;
> +      CHAR16          *FileText;
> +
> +      LoadFilePath = DevicePathFromHandle (LoadFileHandle);
> +      if (LoadFilePath == NULL) {
> +        LoadFileText = NULL;
> +      } else {
> +        LoadFileText = ConvertDevicePathToText (LoadFilePath, FALSE, FALSE);
> +      }
> +      FileText = ConvertDevicePathToText (FilePath, FALSE, FALSE);
> +
> +      DEBUG ((
> +        DEBUG_ERROR,
> +        "%a:%a: failed to allocate reserved pages: "
> +        "BufferSize=%Lu LoadFile=\"%s\" FilePath=\"%s\"\n",
> +        gEfiCallerBaseName,
> +        __FUNCTION__,
> +        (UINT64)BufferSize,
> +        LoadFileText,
> +        FileText
> +        ));
> +
> +      if (FileText != NULL) {
> +        FreePool (FileText);
> +      }
> +      if (LoadFileText != NULL) {
> +        FreePool (LoadFileText);
> +      }
> +      );
>      return NULL;
>    }
> 
> --
> 2.19.1.3.g30247aa5d201
> 
> 
> 
> 

Re: [edk2-devel] [PATCH 0/2] UefiBootManagerLib, HttpDxe: tweaks for large HTTP(S) downloads

[Laszlo Ersek]

On 01/09/20 00:43, Laszlo Ersek wrote:
> Repo:   https://github.com/lersek/edk2.git
> Branch: tweaks_for_large_http
> 
> This series aims to improve HTTP(S) Boot experience with large (4GiB+)
> files.
> 
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Siyuan Fu <siyuan.fu@intel.com>
> Cc: Zhichao Gao <zhichao.gao@intel.com>
> 
> Thanks,
> Laszlo
> 
> Laszlo Ersek (2):
>   MdeModulePkg/UefiBootManagerLib: log reserved mem allocation failure
>   NetworkPkg/HttpDxe: fix 32-bit truncation in HTTPS download
> 
>  MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 31 ++++++++++++++++++++
>  NetworkPkg/HttpDxe/HttpImpl.c                    |  2 +-
>  2 files changed, 32 insertions(+), 1 deletion(-)
> 

Merged: commit range b112ec225f1c..4cca7923992a.

Thanks!
Laszlo