From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: [GIT PULL] for-2.6.32/bug-fixes Date: Tue, 17 May 2011 12:43:59 -0400 Message-ID: <20110517164359.GB28286@dumpdata.com> References: <20110516203535.GA871@dumpdata.com> <4DD260700200007800041962@vpn.id2.novell.com> <20110517141629.GC6816@dumpdata.com> <4DD2AAF80200007800041AC9@vpn.id2.novell.com> <20110517155746.GB3657@dumpdata.com> <4DD2BD6B0200007800041B40@vpn.id2.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <4DD2BD6B0200007800041B40@vpn.id2.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Jan Beulich Cc: Jeremy Fitzhardinge , xen-devel@lists.xensource.com List-Id: xen-devel@lists.xenproject.org On Tue, May 17, 2011 at 05:24:43PM +0100, Jan Beulich wrote: > >>> On 17.05.11 at 17:57, Konrad Rzeszutek Wilk wrote: > >> > No attaching of data to the barrier. > >> > >> Sure, this direction we agree about. But your change is enforcing > >> it the other way around (if barrier then no data), which wasn't the > >> case so far. > > > > OK, even if the code that actually does the bio submission does > > not attach any data to the bio? The end result is the same - no > > data with barriers. > > My problem is that I can't see where attaching data would be > skipped. The only thing I see is the BUG_ON() you pointed at Well, req->ns_segments = 0, so nseg is zero, which means all of those for loops never get executed. > earlier, checking that if there is no data, then this must be a > barrier request. > > >> >> Additionally, looking at the check in vbd_translate(), wouldn't you > >> >> think there ought to be overflow checking for the addition, too? > >> > > >> > Sure, could add that in. Albeit it seems incorrect to do it in that > >> > function. It checks to see if the sector is correct, and -1 is definitly > >> > wrong. > >> > >> Hmm, depends on your perspective - I'd say that any sector_number > >> is valid when nr_sects is zero. > > > > I concur. The value that is passed by the frontend is not zero. It is -1. > > Oh, you say both sector_number and nr_sects are -1? Looking > again... No, that can't be the case, the value starts out at zero > in dispatch_rw_block_io(). No, wait. Argh. The req->nr_sects is 0 and req->sector_number is -1. This is what I got before the fix: access denied: write of [18446744073709551615,18446744073709551615] on dev=ca0 And req->nr_segments is 0.