Re: [PATCH v6 3/8] x86/microcode/AMD: Check microcode container data in the early loader

[Borislav Petkov]

On Thu, Jun 14, 2018 at 10:56:07PM +0200, Maciej S. Szmigiero wrote:
> At this point we don't know the CPU family the particular patch is for
> since the patch header contains only CPU rev_id, not an explicit family
> number.

patch_fam = 0xf + (mc->processor_rev_id >> 12);

which means, you can do

if (patch_fam != family)
return 0;

like verify_and_add_patch() does before calling verify_patch() with the
correct family of the current CPU.

-- 
Regards/Gruss,
Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH v6 3/8] x86/microcode/AMD: Check microcode container data in the early loader

[Borislav Petkov]

On Thu, Jun 14, 2018 at 10:56:07PM +0200, Maciej S. Szmigiero wrote:
> At this point we don't know the CPU family the particular patch is for
> since the patch header contains only CPU rev_id, not an explicit family
> number.

patch_fam = 0xf + (mc->processor_rev_id >> 12);

which means, you can do

if (patch_fam != family)
return 0;

like verify_and_add_patch() does before calling verify_patch() with the
correct family of the current CPU.

-- 
Regards/Gruss,
Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH v6 3/8] x86/microcode/AMD: Check microcode container data in the early loader

[Maciej S. Szmigiero]

On 05.06.2018 10:54, Borislav Petkov wrote:
(..)
>> @@ -258,25 +265,27 @@ static ssize_t parse_container(u8 *ucode, ssize_t 
>> size, struct cont_desc *desc)
>>  
>>  hdr = (u32 *)buf;
>>  
>> -if (hdr[0] != UCODE_UCODE_TYPE)
>> +if (!verify_patch_section(buf, size, true))
>>  break;
>>  
>> -/* Sanity-check patch size. */
>>  patch_size = hdr[1];
>> -if (patch_size > PATCH_MAX_SIZE)
>> -break;
>>  
>> -/* Skip patch section header: */
>> -buf  += SECTION_HDR_SIZE;
>> -size -= SECTION_HDR_SIZE;
>> +mc = (struct microcode_amd *)(buf + SECTION_HDR_SIZE);
>> +if (eq_id != mc->hdr.processor_rev_id)
>> +goto next_patch;
>>  
>> -mc = (struct microcode_amd *)buf;
>> -if (eq_id == mc->hdr.processor_rev_id) {
>> -desc->psize = patch_size;
>> -desc->mc = mc;
>> -}
>> +if (!verify_patch(x86_family(desc->cpuid_1_eax), buf, size,
>> +  true))
> 
> Let it stick out.
> 
> Ok, so above you do verify_patch_section() and then you take patch_size
> without fully verifying it - it can be something non-sensically huge and
> thus we might skip over good patches.
> 
> What you should do instead is call verify_patch() directly - which
> already calls verify_patch_section() and if the patch size exceeds the
> per-family maximum, return *that* instead and skip only the per family
> maximum inside the buffer so that any patches following can get a chance
> to get inspected.

verify_patch_section() does only very basic patch section checks -
more or less whether the section, its header and a patch header exist and
can be accessed at all.

Here, a check can be added whether the indicated patch size does not
exceed PATCH_MAX_SIZE to catch nonsensically huge patch sizes and to
skip only a maximum patch length of that many bytes.

At this point we don't know the CPU family the particular patch is for
since the patch header contains only CPU rev_id, not an explicit family
number.

Only the late loader is able to translate back this CPU rev_id to its
family number via the CPU equivalence table, the early loader simply
skips patches that have CPU rev_id different from the current CPU one -
so it can always use the current CPU family number for a patch
verification.

But either way, in order to read the CPU rev_id in the patch
header we have to verify its presence in the microcode container file.
This is what verify_patch_section() does.

Then, once the particular loader determines the family number for a
patch, verify_patch() does additional per-family size check.
In principle, this function could be renamed verify_patch_family_size()
and have call to verify_patch_section() at its beginning dropped.

Maciej

Re: [PATCH v6 3/8] x86/microcode/AMD: Check microcode container data in the early loader

[Maciej S. Szmigiero]

On 05.06.2018 10:54, Borislav Petkov wrote:
(..)
>> @@ -258,25 +265,27 @@ static ssize_t parse_container(u8 *ucode, ssize_t 
>> size, struct cont_desc *desc)
>>  
>>  hdr = (u32 *)buf;
>>  
>> -if (hdr[0] != UCODE_UCODE_TYPE)
>> +if (!verify_patch_section(buf, size, true))
>>  break;
>>  
>> -/* Sanity-check patch size. */
>>  patch_size = hdr[1];
>> -if (patch_size > PATCH_MAX_SIZE)
>> -break;
>>  
>> -/* Skip patch section header: */
>> -buf  += SECTION_HDR_SIZE;
>> -size -= SECTION_HDR_SIZE;
>> +mc = (struct microcode_amd *)(buf + SECTION_HDR_SIZE);
>> +if (eq_id != mc->hdr.processor_rev_id)
>> +goto next_patch;
>>  
>> -mc = (struct microcode_amd *)buf;
>> -if (eq_id == mc->hdr.processor_rev_id) {
>> -desc->psize = patch_size;
>> -desc->mc = mc;
>> -}
>> +if (!verify_patch(x86_family(desc->cpuid_1_eax), buf, size,
>> +  true))
> 
> Let it stick out.
> 
> Ok, so above you do verify_patch_section() and then you take patch_size
> without fully verifying it - it can be something non-sensically huge and
> thus we might skip over good patches.
> 
> What you should do instead is call verify_patch() directly - which
> already calls verify_patch_section() and if the patch size exceeds the
> per-family maximum, return *that* instead and skip only the per family
> maximum inside the buffer so that any patches following can get a chance
> to get inspected.

verify_patch_section() does only very basic patch section checks -
more or less whether the section, its header and a patch header exist and
can be accessed at all.

Here, a check can be added whether the indicated patch size does not
exceed PATCH_MAX_SIZE to catch nonsensically huge patch sizes and to
skip only a maximum patch length of that many bytes.

At this point we don't know the CPU family the particular patch is for
since the patch header contains only CPU rev_id, not an explicit family
number.

Only the late loader is able to translate back this CPU rev_id to its
family number via the CPU equivalence table, the early loader simply
skips patches that have CPU rev_id different from the current CPU one -
so it can always use the current CPU family number for a patch
verification.

But either way, in order to read the CPU rev_id in the patch
header we have to verify its presence in the microcode container file.
This is what verify_patch_section() does.

Then, once the particular loader determines the family number for a
patch, verify_patch() does additional per-family size check.
In principle, this function could be renamed verify_patch_family_size()
and have call to verify_patch_section() at its beginning dropped.

Maciej

Re: [PATCH v6 3/8] x86/microcode/AMD: Check microcode container data in the early loader

[Borislav Petkov]

On Sun, May 20, 2018 at 12:07:17AM +0200, Maciej S. Szmigiero wrote:
> Convert the early loader in the AMD microcode update driver to use the
> container data checking functions introduced by the previous commit.
> 
> We have to be careful to call these functions with 'early' parameter set,
> so they won't try to print errors as the early loader runs too early for
> printk()-style functions to work.
> 
> Signed-off-by: Maciej S. Szmigiero 
> ---
>  arch/x86/kernel/cpu/microcode/amd.c | 59 -
>  1 file changed, 33 insertions(+), 26 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/microcode/amd.c 
> b/arch/x86/kernel/cpu/microcode/amd.c
> index f9485ff7183c..f4c7479a961c 100644
> --- a/arch/x86/kernel/cpu/microcode/amd.c
> +++ b/arch/x86/kernel/cpu/microcode/amd.c
> @@ -224,29 +224,36 @@ static bool verify_patch(u8 family, const u8 *buf, 
> size_t buf_size, bool early)
>   * Returns the amount of bytes consumed while scanning. @desc contains all 
> the
>   * data we're going to use in later stages of the application.
>   */
> -static ssize_t parse_container(u8 *ucode, ssize_t size, struct cont_desc 
> *desc)
> +static size_t parse_container(u8 *ucode, size_t size, struct cont_desc *desc)
>  {
>   struct equiv_cpu_entry *eq;
> - ssize_t orig_size = size;
> + size_t orig_size = size;
>   u32 *hdr = (u32 *)ucode;
> + u32 equiv_tbl_len;
>   u16 eq_id;
>   u8 *buf;
>  
> - /* Am I looking at an equivalence table header? */
> - if (hdr[0] != UCODE_MAGIC ||
> - hdr[1] != UCODE_EQUIV_CPU_TABLE_TYPE ||
> - hdr[2] == 0)
> - return CONTAINER_HDR_SZ;
> + /*
> +  * Skip one byte when a container cannot be parsed successfully
> +  * so the parser will correctly skip unknown data of any size until
> +  * it hopefully arrives at something that it is able to recognize.
> +  */
> + if (!verify_container(ucode, size, true) ||
> + !verify_equivalence_table(ucode, size, true))

That function already calls verify_container().

> + return 1;
>  
>   buf = ucode;
>  
> + equiv_tbl_len = hdr[2];
>   eq = (struct equiv_cpu_entry *)(buf + CONTAINER_HDR_SZ);
>  
>   /* Find the equivalence ID of our CPU in this table: */
>   eq_id = find_equiv_id(eq, desc->cpuid_1_eax);
>  
> - buf  += hdr[2] + CONTAINER_HDR_SZ;
> - size -= hdr[2] + CONTAINER_HDR_SZ;
> + buf  += CONTAINER_HDR_SZ;
> + buf  += equiv_tbl_len;
> + size -= CONTAINER_HDR_SZ;
> + size -= equiv_tbl_len;
>  
>   /*
>* Scan through the rest of the container to find where it ends. We do
> @@ -258,25 +265,27 @@ static ssize_t parse_container(u8 *ucode, ssize_t size, 
> struct cont_desc *desc)
>  
>   hdr = (u32 *)buf;
>  
> - if (hdr[0] != UCODE_UCODE_TYPE)
> + if (!verify_patch_section(buf, size, true))
>   break;
>  
> - /* Sanity-check patch size. */
>   patch_size = hdr[1];
> - if (patch_size > PATCH_MAX_SIZE)
> - break;
>  
> - /* Skip patch section header: */
> - buf  += SECTION_HDR_SIZE;
> - size -= SECTION_HDR_SIZE;
> + mc = (struct microcode_amd *)(buf + SECTION_HDR_SIZE);
> + if (eq_id != mc->hdr.processor_rev_id)
> + goto next_patch;
>  
> - mc = (struct microcode_amd *)buf;
> - if (eq_id == mc->hdr.processor_rev_id) {
> - desc->psize = patch_size;
> - desc->mc = mc;
> - }
> + if (!verify_patch(x86_family(desc->cpuid_1_eax), buf, size,
> +   true))

Let it stick out.

Ok, so above you do verify_patch_section() and then you take patch_size
without fully verifying it - it can be something non-sensically huge and
thus we might skip over good patches.

What you should do instead is call verify_patch() directly - which
already calls verify_patch_section() and if the patch size exceeds the
per-family maximum, return *that* instead and skip only the per family
maximum inside the buffer so that any patches following can get a chance
to get inspected.

For that you'll have to reshuffle the change of integrating
verify_patch_size() to happen before that change here.

Thx.

-- 
Regards/Gruss,
Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH v6 3/8] x86/microcode/AMD: Check microcode container data in the early loader

[Borislav Petkov]

On Sun, May 20, 2018 at 12:07:17AM +0200, Maciej S. Szmigiero wrote:
> Convert the early loader in the AMD microcode update driver to use the
> container data checking functions introduced by the previous commit.
> 
> We have to be careful to call these functions with 'early' parameter set,
> so they won't try to print errors as the early loader runs too early for
> printk()-style functions to work.
> 
> Signed-off-by: Maciej S. Szmigiero 
> ---
>  arch/x86/kernel/cpu/microcode/amd.c | 59 -
>  1 file changed, 33 insertions(+), 26 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/microcode/amd.c 
> b/arch/x86/kernel/cpu/microcode/amd.c
> index f9485ff7183c..f4c7479a961c 100644
> --- a/arch/x86/kernel/cpu/microcode/amd.c
> +++ b/arch/x86/kernel/cpu/microcode/amd.c
> @@ -224,29 +224,36 @@ static bool verify_patch(u8 family, const u8 *buf, 
> size_t buf_size, bool early)
>   * Returns the amount of bytes consumed while scanning. @desc contains all 
> the
>   * data we're going to use in later stages of the application.
>   */
> -static ssize_t parse_container(u8 *ucode, ssize_t size, struct cont_desc 
> *desc)
> +static size_t parse_container(u8 *ucode, size_t size, struct cont_desc *desc)
>  {
>   struct equiv_cpu_entry *eq;
> - ssize_t orig_size = size;
> + size_t orig_size = size;
>   u32 *hdr = (u32 *)ucode;
> + u32 equiv_tbl_len;
>   u16 eq_id;
>   u8 *buf;
>  
> - /* Am I looking at an equivalence table header? */
> - if (hdr[0] != UCODE_MAGIC ||
> - hdr[1] != UCODE_EQUIV_CPU_TABLE_TYPE ||
> - hdr[2] == 0)
> - return CONTAINER_HDR_SZ;
> + /*
> +  * Skip one byte when a container cannot be parsed successfully
> +  * so the parser will correctly skip unknown data of any size until
> +  * it hopefully arrives at something that it is able to recognize.
> +  */
> + if (!verify_container(ucode, size, true) ||
> + !verify_equivalence_table(ucode, size, true))

That function already calls verify_container().

> + return 1;
>  
>   buf = ucode;
>  
> + equiv_tbl_len = hdr[2];
>   eq = (struct equiv_cpu_entry *)(buf + CONTAINER_HDR_SZ);
>  
>   /* Find the equivalence ID of our CPU in this table: */
>   eq_id = find_equiv_id(eq, desc->cpuid_1_eax);
>  
> - buf  += hdr[2] + CONTAINER_HDR_SZ;
> - size -= hdr[2] + CONTAINER_HDR_SZ;
> + buf  += CONTAINER_HDR_SZ;
> + buf  += equiv_tbl_len;
> + size -= CONTAINER_HDR_SZ;
> + size -= equiv_tbl_len;
>  
>   /*
>* Scan through the rest of the container to find where it ends. We do
> @@ -258,25 +265,27 @@ static ssize_t parse_container(u8 *ucode, ssize_t size, 
> struct cont_desc *desc)
>  
>   hdr = (u32 *)buf;
>  
> - if (hdr[0] != UCODE_UCODE_TYPE)
> + if (!verify_patch_section(buf, size, true))
>   break;
>  
> - /* Sanity-check patch size. */
>   patch_size = hdr[1];
> - if (patch_size > PATCH_MAX_SIZE)
> - break;
>  
> - /* Skip patch section header: */
> - buf  += SECTION_HDR_SIZE;
> - size -= SECTION_HDR_SIZE;
> + mc = (struct microcode_amd *)(buf + SECTION_HDR_SIZE);
> + if (eq_id != mc->hdr.processor_rev_id)
> + goto next_patch;
>  
> - mc = (struct microcode_amd *)buf;
> - if (eq_id == mc->hdr.processor_rev_id) {
> - desc->psize = patch_size;
> - desc->mc = mc;
> - }
> + if (!verify_patch(x86_family(desc->cpuid_1_eax), buf, size,
> +   true))

Let it stick out.

Ok, so above you do verify_patch_section() and then you take patch_size
without fully verifying it - it can be something non-sensically huge and
thus we might skip over good patches.

What you should do instead is call verify_patch() directly - which
already calls verify_patch_section() and if the patch size exceeds the
per-family maximum, return *that* instead and skip only the per family
maximum inside the buffer so that any patches following can get a chance
to get inspected.

For that you'll have to reshuffle the change of integrating
verify_patch_size() to happen before that change here.

Thx.

-- 
Regards/Gruss,
Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[PATCH v6 3/8] x86/microcode/AMD: Check microcode container data in the early loader

[Maciej S. Szmigiero]

Convert the early loader in the AMD microcode update driver to use the
container data checking functions introduced by the previous commit.

We have to be careful to call these functions with 'early' parameter set,
so they won't try to print errors as the early loader runs too early for
printk()-style functions to work.

Signed-off-by: Maciej S. Szmigiero 
---
 arch/x86/kernel/cpu/microcode/amd.c | 59 -
 1 file changed, 33 insertions(+), 26 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/amd.c 
b/arch/x86/kernel/cpu/microcode/amd.c
index f9485ff7183c..f4c7479a961c 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -224,29 +224,36 @@ static bool verify_patch(u8 family, const u8 *buf, size_t 
buf_size, bool early)
  * Returns the amount of bytes consumed while scanning. @desc contains all the
  * data we're going to use in later stages of the application.
  */
-static ssize_t parse_container(u8 *ucode, ssize_t size, struct cont_desc *desc)
+static size_t parse_container(u8 *ucode, size_t size, struct cont_desc *desc)
 {
struct equiv_cpu_entry *eq;
-   ssize_t orig_size = size;
+   size_t orig_size = size;
u32 *hdr = (u32 *)ucode;
+   u32 equiv_tbl_len;
u16 eq_id;
u8 *buf;
 
-   /* Am I looking at an equivalence table header? */
-   if (hdr[0] != UCODE_MAGIC ||
-   hdr[1] != UCODE_EQUIV_CPU_TABLE_TYPE ||
-   hdr[2] == 0)
-   return CONTAINER_HDR_SZ;
+   /*
+* Skip one byte when a container cannot be parsed successfully
+* so the parser will correctly skip unknown data of any size until
+* it hopefully arrives at something that it is able to recognize.
+*/
+   if (!verify_container(ucode, size, true) ||
+   !verify_equivalence_table(ucode, size, true))
+   return 1;
 
buf = ucode;
 
+   equiv_tbl_len = hdr[2];
eq = (struct equiv_cpu_entry *)(buf + CONTAINER_HDR_SZ);
 
/* Find the equivalence ID of our CPU in this table: */
eq_id = find_equiv_id(eq, desc->cpuid_1_eax);
 
-   buf  += hdr[2] + CONTAINER_HDR_SZ;
-   size -= hdr[2] + CONTAINER_HDR_SZ;
+   buf  += CONTAINER_HDR_SZ;
+   buf  += equiv_tbl_len;
+   size -= CONTAINER_HDR_SZ;
+   size -= equiv_tbl_len;
 
/*
 * Scan through the rest of the container to find where it ends. We do
@@ -258,25 +265,27 @@ static ssize_t parse_container(u8 *ucode, ssize_t size, 
struct cont_desc *desc)
 
hdr = (u32 *)buf;
 
-   if (hdr[0] != UCODE_UCODE_TYPE)
+   if (!verify_patch_section(buf, size, true))
break;
 
-   /* Sanity-check patch size. */
patch_size = hdr[1];
-   if (patch_size > PATCH_MAX_SIZE)
-   break;
 
-   /* Skip patch section header: */
-   buf  += SECTION_HDR_SIZE;
-   size -= SECTION_HDR_SIZE;
+   mc = (struct microcode_amd *)(buf + SECTION_HDR_SIZE);
+   if (eq_id != mc->hdr.processor_rev_id)
+   goto next_patch;
 
-   mc = (struct microcode_amd *)buf;
-   if (eq_id == mc->hdr.processor_rev_id) {
-   desc->psize = patch_size;
-   desc->mc = mc;
-   }
+   if (!verify_patch(x86_family(desc->cpuid_1_eax), buf, size,
+ true))
+   goto next_patch;
+
+   /* We have found a matching patch! */
+   desc->psize = patch_size;
+   desc->mc = mc;
 
+next_patch:
+   buf  += SECTION_HDR_SIZE;
buf  += patch_size;
+   size -= SECTION_HDR_SIZE;
size -= patch_size;
}
 
@@ -303,15 +312,13 @@ static ssize_t parse_container(u8 *ucode, ssize_t size, 
struct cont_desc *desc)
  */
 static void scan_containers(u8 *ucode, size_t size, struct cont_desc *desc)
 {
-   ssize_t rem = size;
-
-   while (rem >= 0) {
-   ssize_t s = parse_container(ucode, rem, desc);
+   while (size > 0) {
+   size_t s = parse_container(ucode, size, desc);
if (!s)
return;
 
ucode += s;
-   rem   -= s;
+   size  -= s;
}
 }
 

[PATCH v6 3/8] x86/microcode/AMD: Check microcode container data in the early loader

[Maciej S. Szmigiero]

Convert the early loader in the AMD microcode update driver to use the
container data checking functions introduced by the previous commit.

We have to be careful to call these functions with 'early' parameter set,
so they won't try to print errors as the early loader runs too early for
printk()-style functions to work.

Signed-off-by: Maciej S. Szmigiero 
---
 arch/x86/kernel/cpu/microcode/amd.c | 59 -
 1 file changed, 33 insertions(+), 26 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/amd.c 
b/arch/x86/kernel/cpu/microcode/amd.c
index f9485ff7183c..f4c7479a961c 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -224,29 +224,36 @@ static bool verify_patch(u8 family, const u8 *buf, size_t 
buf_size, bool early)
  * Returns the amount of bytes consumed while scanning. @desc contains all the
  * data we're going to use in later stages of the application.
  */
-static ssize_t parse_container(u8 *ucode, ssize_t size, struct cont_desc *desc)
+static size_t parse_container(u8 *ucode, size_t size, struct cont_desc *desc)
 {
struct equiv_cpu_entry *eq;
-   ssize_t orig_size = size;
+   size_t orig_size = size;
u32 *hdr = (u32 *)ucode;
+   u32 equiv_tbl_len;
u16 eq_id;
u8 *buf;
 
-   /* Am I looking at an equivalence table header? */
-   if (hdr[0] != UCODE_MAGIC ||
-   hdr[1] != UCODE_EQUIV_CPU_TABLE_TYPE ||
-   hdr[2] == 0)
-   return CONTAINER_HDR_SZ;
+   /*
+* Skip one byte when a container cannot be parsed successfully
+* so the parser will correctly skip unknown data of any size until
+* it hopefully arrives at something that it is able to recognize.
+*/
+   if (!verify_container(ucode, size, true) ||
+   !verify_equivalence_table(ucode, size, true))
+   return 1;
 
buf = ucode;
 
+   equiv_tbl_len = hdr[2];
eq = (struct equiv_cpu_entry *)(buf + CONTAINER_HDR_SZ);
 
/* Find the equivalence ID of our CPU in this table: */
eq_id = find_equiv_id(eq, desc->cpuid_1_eax);
 
-   buf  += hdr[2] + CONTAINER_HDR_SZ;
-   size -= hdr[2] + CONTAINER_HDR_SZ;
+   buf  += CONTAINER_HDR_SZ;
+   buf  += equiv_tbl_len;
+   size -= CONTAINER_HDR_SZ;
+   size -= equiv_tbl_len;
 
/*
 * Scan through the rest of the container to find where it ends. We do
@@ -258,25 +265,27 @@ static ssize_t parse_container(u8 *ucode, ssize_t size, 
struct cont_desc *desc)
 
hdr = (u32 *)buf;
 
-   if (hdr[0] != UCODE_UCODE_TYPE)
+   if (!verify_patch_section(buf, size, true))
break;
 
-   /* Sanity-check patch size. */
patch_size = hdr[1];
-   if (patch_size > PATCH_MAX_SIZE)
-   break;
 
-   /* Skip patch section header: */
-   buf  += SECTION_HDR_SIZE;
-   size -= SECTION_HDR_SIZE;
+   mc = (struct microcode_amd *)(buf + SECTION_HDR_SIZE);
+   if (eq_id != mc->hdr.processor_rev_id)
+   goto next_patch;
 
-   mc = (struct microcode_amd *)buf;
-   if (eq_id == mc->hdr.processor_rev_id) {
-   desc->psize = patch_size;
-   desc->mc = mc;
-   }
+   if (!verify_patch(x86_family(desc->cpuid_1_eax), buf, size,
+ true))
+   goto next_patch;
+
+   /* We have found a matching patch! */
+   desc->psize = patch_size;
+   desc->mc = mc;
 
+next_patch:
+   buf  += SECTION_HDR_SIZE;
buf  += patch_size;
+   size -= SECTION_HDR_SIZE;
size -= patch_size;
}
 
@@ -303,15 +312,13 @@ static ssize_t parse_container(u8 *ucode, ssize_t size, 
struct cont_desc *desc)
  */
 static void scan_containers(u8 *ucode, size_t size, struct cont_desc *desc)
 {
-   ssize_t rem = size;
-
-   while (rem >= 0) {
-   ssize_t s = parse_container(ucode, rem, desc);
+   while (size > 0) {
+   size_t s = parse_container(ucode, size, desc);
if (!s)
return;
 
ucode += s;
-   rem   -= s;
+   size  -= s;
}
 }