Re: [Xen-devel] [PATCH V2] x86/mm: Fix Coverity issues 1373105 and 1373106

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 21/09/16 13:41, Razvan Cojocaru wrote:
> Added missing error checks in p2m_set_mem_access_multi().
> 
> Signed-off-by: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
> ---
> Changes since V1:
>  - Returning -EFAULT instead of -EINVAL.
>  - Replaced stray TAB with spaces.
> ---
>  xen/arch/x86/mm/p2m.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> index b16e563..9526fff 100644
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -1936,8 +1936,12 @@ long p2m_set_mem_access_multi(struct domain *d,
>          uint8_t access;
>          uint64_t gfn_l;
>  
> -        copy_from_guest_offset(&gfn_l, pfn_list, start, 1);
> -        copy_from_guest_offset(&access, access_list, start, 1);
> +        if ( copy_from_guest_offset(&gfn_l, pfn_list, start, 1) ||
> +             copy_from_guest_offset(&access, access_list, start, 1) )
> +        {
> +            rc = -EFAULT;
> +            break;
> +        }

This will return EFAULT even if it has managed to successfully handle
some of the pfn/access pairs.  It looks like this is sort of typical
(the handful of places I could find that had copy_from_guest* inside a
loop followed a similar form).

So:

Reviewed-by: George Dunlap <george.dunlap@xxxxxxxxxx>

I'll check this in.

 -George


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