Re: [Xen-devel] [Patch] x86/mm: Prevent leaking domain mappings in paging_log_dirty_op()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 10.12.13 at 14:53, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
> Coverity ID: 1135374 1135375 1135376 1135377
> 
> If {copy_to,clear}_guest_offset() fails, we would leak the domain mappings 
> for
> l4 thru l1.
> 
> Fixing this requires having conditional unmaps on the faulting path, which 
> in
> turn requires explicitly initialising the pointers to NULL because of the
> early ENOMEM exit.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

with a minor comment:

> @@ -432,6 +432,15 @@ int paging_log_dirty_op(struct domain *d, struct 
> xen_domctl_shadow_op *sc)
>      return rv;
>  
>   out:
> +    if ( l1 )
> +        unmap_domain_page(l1);
> +    if ( l2 )
> +        unmap_domain_page(l2);
> +    if ( l3 )
> +        unmap_domain_page(l3);
> +    if ( l4 )
> +        unmap_domain_page(l4);
> +
>      paging_unlock(d);
>      domain_unpause(d);
>      return rv;

While on an error path, it would nevertheless seem better to do the
unmaps after the unlock/unpause.

Jan


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