Re: [Xen-devel] [PATCH 05/11] tmem: don't access guest memory without using the accessors intended for this

[Dan Magenheimer <dan.magenheimer@xxxxxxxxxx>]

> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: Wednesday, September 05, 2012 6:37 AM
> To: xen-devel
> Cc: Dan Magenheimer; Zhenzhong Duan
> Subject: [PATCH 05/11] tmem: don't access guest memory without using the 
> accessors intended for this
> 
> This is not permitted, not even for buffers coming from Dom0 (and it
> would also break the moment Dom0 runs in HVM mode). An implication from
> the changes here is that tmh_copy_page() can't be used anymore for
> control operations calling tmh_copy_{from,to}_client() (as those pass
> the buffer by virtual address rather than MFN).
> 
> Note that tmemc_save_get_next_page() previously didn't set the returned
> handle's pool_id field, while the new code does. It need to be
> confirmed that this is not a problem (otherwise the copy-out operation
> will require further tmh_...() abstractions to be added).
> 
> Further note that the patch removes (rather than adjusts) an invalid
> call to unmap_domain_page() (no matching map_domain_page()) from
> tmh_compress_from_client() and adds a missing one to an error return
> path in tmh_copy_from_client().
> 
> Finally note that the patch adds a previously missing return statement
> to cli_get_page() (without which that function could de-reference a
> NULL pointer, triggerable from guest mode).
> 
> This is part of XSA-15 / CVE-2012-3497.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

I'm a bit baffled by all the unrelated changes and cleanups, but
they all appear to be valid fixes and, most importantly, the
related security issues appear to be resolved.

I'm also unable right now to plumb the depths of the guest copying
macros so will have to trust Jan's good judgement.  One point in
particular, it's difficult to determine if the following line is
copying two bytes (wrong) or two uint64_t's (correct).

> +        tmh_copy_to_client_buf(buf, pool->uuid, 2);

Last, this patch almost certainly breaks save/restore/migration of
tmem-enabled guests, but that functionality (which once worked
fine) has apparently been broken for awhile.  So let's go
with this new code and fix save/restore/migration on top of it.

Acked-by: Dan Magenheimer <dan.magenheimer@xxxxxxxxxx>

(no further comments in the code, so patch elided)

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