Re: [Xen-devel] [PATCH v5] dmar: device scope mem leak fix

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 01.07.15 at 20:30, <elena.ufimtseva@xxxxxxxxxx> wrote:
> Release memory allocated for scope.devices when disabling
> dmar units. Also set device count after memory allocation when
> device scope parsing.
> This is explanation of why the code should be moved imho and
> answers Jan question about why I needed to do this.
> In acpi_parse_one_drhr move call to acpi_parse_dev_scope after include_all
> check so the return value does not get overwritten by calling 
> acpi_parse_dev_scope.

I can't really connect the middle paragraph to the first or last one,
and in any event this doesn't seem to belong in a commit message
in that shape. Nor can I see the reason for the movement, even
with the last sentence above trying to explain it. What return value
do you see being overwritten? And how does that relate to the
intention of this patch?

> @@ -474,12 +486,10 @@ acpi_parse_one_drhd(struct acpi_dmar_header *header)
>  
>      ret = iommu_alloc(dmaru);
>      if ( ret )
> -        goto out;
> -
> -    dev_scope_start = (void *)(drhd + 1);
> -    dev_scope_end = ((void *)drhd) + header->length;
> -    ret = acpi_parse_dev_scope(dev_scope_start, dev_scope_end,
> -                               &dmaru->scope, DMAR_TYPE, drhd->segment);
> +    {
> +        xfree(dmaru);
> +        return ret;
> +    }    

Why is this being changed from "goto out"? You're now possibly
leaking memory as well as a mapping if iommu_alloc() failed on any
of its actions after having set drhd->iommu.

Jan


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