Re: [GIT PULL] for-2.6.32/bug-fixes

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Tue, May 17, 2011 at 05:24:43PM +0100, Jan Beulich wrote:
> >>> On 17.05.11 at 17:57, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> 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.