Re: [Patch v2 3/4] AMD/microcode: Avoid use-after-free for the microcode buffer

[Andrew Cooper <andrew.cooper3@citrix.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2487 bytes --]

On 27/12/2013 22:43, Matthew Daley wrote:
> On Sat, Dec 28, 2013 at 4:57 AM, Andrew Cooper
> <andrew.cooper3@citrix.com <mailto:andrew.cooper3@citrix.com>> wrote:
>
>     It is possible to free the mc_old buffer and then store it for use
>     in the case
>     of resume.
>
>     This keeps the old semantics of being able to return an error even
>     after a
>     successful microcode application.
>
>     Coverity-ID 1146953
>     Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com
>     <mailto:andrew.cooper3@citrix.com>>
>     CC: Keir Fraser <keir@xen.org <mailto:keir@xen.org>>
>     CC: Jan Beulich <JBeulich@suse.com <mailto:JBeulich@suse.com>>
>     Acked-by: Boris Ostrovsky <boris.ostrovsky@oracle.com
>     <mailto:boris.ostrovsky@oracle.com>>
>     CC: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com
>     <mailto:suravee.suthikulpanit@amd.com>>
>
>     ---
>     Changes in v2:
>      * Fix commit message.  No code change.
>     ---
>      xen/arch/x86/microcode_amd.c |   17 +++++++++--------
>      1 file changed, 9 insertions(+), 8 deletions(-)
>
>     diff --git a/xen/arch/x86/microcode_amd.c
>     b/xen/arch/x86/microcode_amd.c
>     index a3ceef8..8ea4e63 100644
>     --- a/xen/arch/x86/microcode_amd.c
>     +++ b/xen/arch/x86/microcode_amd.c
>     @@ -275,7 +275,7 @@ static int cpu_request_microcode(int cpu,
>     const void *buf, size_t bufsize)
>          struct microcode_amd *mc_amd, *mc_old;
>          size_t offset = bufsize;
>          size_t last_offset, applied_offset = 0;
>     -    int error = 0;
>     +    int error = 0, save_error = 1;
>          struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu);
>
>          /* We should bind the task to the CPU */
>     @@ -338,19 +338,20 @@ static int cpu_request_microcode(int cpu,
>     const void *buf, size_t bufsize)
>           */
>          if ( applied_offset )
>          {
>     -        int ret = get_ucode_from_buffer_amd(mc_amd, buf, bufsize,
>     -                                            &applied_offset);
>     -        if ( ret == 0 )
>     -            xfree(mc_old);
>     -        else
>     -            error = ret;
>     +        save_error = get_ucode_from_buffer_amd(
>     +            mc_amd, buf, bufsize, &applied_offset);
>     +
>     +        if ( save_error )
>     +            error = save_error
>
>
> Missing you-know-what.
>
> - Matthew

I really should learn that "trivial tweaks" are often not quite so
trivial. v3 on its way.

~Andrew

[-- Attachment #1.2: Type: text/html, Size: 5367 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel