lkml.org 
[lkml]   [2013]   [Mar]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [ 34/77] xen/blkback: Dont trust the handle from the frontend.
From
Date
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



\
 
 \ /
  Last update: 2013-03-03 00:01    [W:0.115 / U:0.108 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site