Re: [ 34/77] xen/blkback: Dont trust the handle from the frontend.

From: Paul Bolle
Date: Sat Mar 02 2013 - 17:35:37 EST


On Sat, 2013-03-02 at 19:48 +0000, Ben Hutchings wrote:
> When gcc compiles something like this:
>
> static int foo(int *p)
> {
> if (rand() & 1)
> return -1;
> *p = 0;
> return 0;
> }
>
> int bar(void)
> {
> int i;
> if (foo(&i) < 0)
> return 1;
> return i;
> }
>
> and inlines foo() into bar(), sometimes it fails to recognise that i
> will definitely be initialised before use. This simple example seems to
> be OK but more complex functions such as these will often trigger this
> warning. The warning is really quite useless now.

0) I've had another look at the relevant code in v3.8.2-rc1. It can be
summarized like this:

static int xen_vbd_translate()
{
[...]
int rc = -EACCES;

if ([...])
goto out;
[...]

[p]req->dev = vbd->pdevice;
[p]req->bdev = vbd->bdev;
[...]

out:
return rc;
}

static int dispatch_rw_block_io()
{
struct phys_req preq;
[...]

preq.sector_number = req->u.rw.sector_number;
preq.nr_sects = 0;
[...]

for ([...]) {
[...]
preq.nr_sects += seg[i].nsec;
}

if (xen_vbd_translate(&preq, blkif, operation) != 0) {
pr_debug(DRV_PFX "access denied: %s of [%llu,%llu] on dev=%04x\n",
operation == READ ? "read" : "write",
preq.sector_number,
preq.sector_number + preq.nr_sects, preq.dev);
goto [...];
}
[...]
}

1) So if xen_vbd_translate() fails, it can return before setting
preq.dev. That makes the call of pr_debug() use an uninitialized value,
doesn't it? Does inlining xen_vbd_translate() and/or
dispatch_rw_block_io() generate code were that can't happen anymore?
(Both functions being static they probably are inlined.)

2) And even if inlining does generate code where this can't happen,
isn't it enough that preq.dev can be used uninitialized if no code were
inlined?


Paul Bolle

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/