From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gerd Hoffmann Subject: Re: Two small patches related to xenfb Date: Mon, 29 Sep 2008 11:13:14 +0200 Message-ID: <48E09C2A.6000203@redhat.com> References: <20080926140548.GC31985@emperor2.home.aster.pl> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20080926140548.GC31985@emperor2.home.aster.pl> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Rafal Wojtczuk Cc: xen-devel@lists.xensource.com List-Id: xen-devel@lists.xenproject.org Rafal Wojtczuk wrote: > Hello, > Two minor issues: > row_stride_div0.patch: a malicious frontend can send row_stride==0 and force > qemu-dm to perform division by 0 Ok. > vnc_resize_doublecheck.patch: there is an unchecked multiplication when > calculating framebuffer size. Cs 17630 sanitizes framebuffer dimensions > passed by the frontend, so most probably no integer overflow can happen, but > there should be a check for overflow close to the actual computation (to > make code review easier and to cope with other codepaths in the future). If bogous values can make it through the sanity checks in xenfb_configure_fb() then those sanity checks must be fixed. Adding another check somewhere else certainly doesn't make review easier. In contrast it makes error handling more complicated because there are multiple places where you have to deal with errors instead of just one functions which does all sanity checks. cheers, Gerd