From: Markus Armbruster <armbru@redhat.com>
Subject: Re: Fix PVFB backend to validate frontend's frame buffer
	description
Date: Thu, 15 May 2008 09:40:57 +0200
Message-ID: <87abisauba.fsf@pike.pond.sub.org>
References: <877idyxq1i.fsf@pike.pond.sub.org> <482B29C9.6010007@novell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
In-Reply-To: <482B29C9.6010007@novell.com> (Pat Campbell's message of "Wed\,
	14 May 2008 12\:04\:57 -0600")
Sender: xen-devel-bounces@lists.xensource.com
Errors-To: xen-devel-bounces@lists.xensource.com
To: Pat Campbell <plc@novell.com>
Cc: xen-devel@lists.xensource.com

Pat Campbell <plc@novell.com> writes:

> Markus Armbruster wrote:
>> A buggy or malicious frontend can describe its shared framebuffer to
>> the backend in a way that makes the backend map an arbitrary amount of
>>
>>   
> snipped out, see inline question below.
>>
>> diff -r 0a8fc1a62796 tools/ioemu/hw/xenfb.c
>> --- a/tools/ioemu/hw/xenfb.c	Mon May 12 11:19:09 2008 +0100
>> +++ b/tools/ioemu/hw/xenfb.c	Tue May 13 14:53:58 2008 +0200
>> @@ -28,8 +28,6 @@
[....]
>> +static int xenfb_configure_fb(struct xenfb *xenfb, size_t fb_len_lim,
>> +			      int width, int height, int depth,
>> +			      size_t fb_len, int offset, int row_stride)
>> +{
>> +	size_t mfn_sz = sizeof(*((struct xenfb_page *)0)->pd);
>> +	size_t pd_len = sizeof(((struct xenfb_page *)0)->pd) / mfn_sz;
>> +	size_t fb_pages = pd_len * XC_PAGE_SIZE / mfn_sz;
>> +	size_t fb_len_max = fb_pages * XC_PAGE_SIZE;
>> +	int max_width, max_height;
>> +
>> +	if (fb_len_lim > fb_len_max) {
>> +		fprintf(stderr,
>> +			"FB: fb size limit %zu exceeds %zu, corrected\n",
>> +			fb_len_lim, fb_len_max);
>> +		fb_len_lim = fb_len_max;
>> +	}
>> +	if (fb_len > fb_len_lim) {
>> +		fprintf(stderr,
>> +			"FB: frontend fb size %zu limited to %zu\n",
>> +			fb_len, fb_len_lim);
>>   
> Do we need to set fb_len to fb_len_lim here?
> fb_len = fb_len_lim;

Yes, we do!  Good catch, thank you.  I have no idea how that got lost.
I'll post a patch.

[...]