Re: [Xen-devel] [PATCH 1/2] x86/mm: Avoid NULL dereference when checking altp2m's for shareability

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 27/07/16 19:08, Andrew Cooper wrote:
> Coverity identifies that __get_gfn_type_access() unconditionally writes to its
> type parameter under a number of circumstances.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

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

> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Tim Deegan <tim@xxxxxxx>
> CC: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
> CC: Tamas K Lengyel <tamas.lengyel@xxxxxxxxxxxx>
> 
> There is a second complaint that ap2ma and p2ma are used before initialisation
> in the following line, although that is harder to reason about.  I think the
> code is OK...

Well there are paths through __get_gfn_type_access() which don't set the
access value -- namely if p2m is null or
!paging_mode_translate(p2m->domain) (which coverity has no way of knowing).

That probably could use being made more robust at some point.

 -George

> ---
>  xen/arch/x86/mm/mem_sharing.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
> index 47e0820..14952ce 100644
> --- a/xen/arch/x86/mm/mem_sharing.c
> +++ b/xen/arch/x86/mm/mem_sharing.c
> @@ -870,6 +870,7 @@ int mem_sharing_nominate_page(struct domain *d,
>          unsigned int i;
>          struct p2m_domain *ap2m;
>          mfn_t amfn;
> +        p2m_type_t ap2mt;
>          p2m_access_t ap2ma;
>  
>          altp2m_list_lock(d);
> @@ -880,7 +881,7 @@ int mem_sharing_nominate_page(struct domain *d,
>              if ( !ap2m )
>                  continue;
>  
> -            amfn = get_gfn_type_access(ap2m, gfn, NULL, &ap2ma, 0, NULL);
> +            amfn = get_gfn_type_access(ap2m, gfn, &ap2mt, &ap2ma, 0, NULL);
>              if ( mfn_valid(amfn) && (mfn_x(amfn) != mfn_x(mfn) || ap2ma != 
> p2ma) )
>              {
>                  altp2m_list_unlock(d);
> 


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