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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]