Re: [Xen-devel] [PATCH for-4.5 V6 13/14] Fix freeing of uninitialized pointer

[Roy Franz <roy.franz@xxxxxxxxxx>]

On Wed, Sep 24, 2014 at 2:07 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>> On 24.09.14 at 07:03, <roy.franz@xxxxxxxxxx> wrote:
>> The only valid response from the LocateHandle() call is EFI_BUFFER_TOO_SMALL,
>> so exit if we get anything else.  We pass a 0 size/NULL pointer buffer, so 
>> the
>> only other returns we will get is an error.  Return right away as there is
>> nothing to do.  Also return if there is an error allocating the buffer, as 
>> the
>> previous code path also allowed for an undefined pointer to be freed.
>>
>> Signed-off-by: Roy Franz <roy.franz@xxxxxxxxxx>
>
> Thanks, but I restructured the patch (see below). Additionally such
> bug fixes would better be placed at the start of a series to ease
> backporting.
>
> Jan
>
> x86/EFI: fix freeing of uninitialized pointer
>
> The only valid response from the LocateHandle() call is EFI_BUFFER_TOO_SMALL,
> so exit if we get anything else.  We pass a 0 size/NULL pointer buffer, so the
> only other returns we will get is an error.  Return right away as there is
> nothing to do.  Also return if there is an error allocating the buffer, as the
> previous code path also allowed for an undefined pointer to be freed.
>
> Signed-off-by: Roy Franz <roy.franz@xxxxxxxxxx>
>
> Re-structure the change.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>
> --- a/xen/arch/x86/efi/boot.c
> +++ b/xen/arch/x86/efi/boot.c
> @@ -595,11 +595,12 @@ static void __init setup_efi_pci(void)
>      struct efi_pci_rom *last = NULL;
>
>      status = efi_bs->LocateHandle(ByProtocol, &pci_guid, NULL, &size, NULL);
> -    if ( status == EFI_BUFFER_TOO_SMALL )
> -        status = efi_bs->AllocatePool(EfiLoaderData, size, (void 
> **)&handles);
> -    if ( !EFI_ERROR(status) )
> -        status = efi_bs->LocateHandle(ByProtocol, &pci_guid, NULL, &size,
> -                                      handles);
> +    if ( status != EFI_BUFFER_TOO_SMALL )
> +        return;
> +    status = efi_bs->AllocatePool(EfiLoaderData, size, (void **)&handles);
> +    if ( EFI_ERROR(status) )
> +        return;
> +    status = efi_bs->LocateHandle(ByProtocol, &pci_guid, NULL, &size, 
> handles);
>      if ( EFI_ERROR(status) )
>          size = 0;
>
>
>
>

OK, I'll use your version, and move it to the start of the patch series.

Roy

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