linux-kernel - Re: [PATCH] 3/3 maple: update bus driver to support Dreamcast VMU

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 24 Mar 2008 17:55:51 +0000
From:	Adrian McMenamin <adrian@...golddream.dyndns.info>
To:	Jörn Engel <joern@...fs.org>
Cc:	Paul Mundt <lethal@...ux-sh.org>, Greg KH <greg@...ah.com>,
	dwmw2 <dwmw2@...radead.org>, LKML <linux-kernel@...r.kernel.org>,
	MTD <linux-mtd@...ts.infradead.org>,
	linux-sh <linux-sh@...r.kernel.org>,
	Andrew Morton <akpm@...l.org>
Subject: Re: [PATCH] 3/3 maple: update bus driver to support Dreamcast VMU


On Mon, 2008-03-24 at 18:38 +0100, Jörn Engel wrote:
> On Mon, 24 March 2008 18:07:07 +0100, Jörn Engel wrote:
> > 
> > The second rearranges the list locking a bit.  Previously it was
> > possible to touch maple_waitq or maple_sentq without holding the lock.
> > With my limited understanding of the driver, the second patch may
> > already be enough to prevent the type of corruption you've been seeing.
> 
> Limited understanding indeed.  The problem in the mtd driver is that it
> has no concept of ownership.  For example maple_vmu_read_block() does
> the following:
> 	mdev->mq->sendbuf = sendbuf;
> 	...
> 	maple_add_packet(mdev->mq);
> 
> It accesses some field in mdev->mq, then sends the structure off to
> maple_add_packet().  From this point on, mdev->mq belongs to the bus
> driver - until it calls the callback, vmu_blockread() in this case.
> 
> But a second thread can call into maple_vmu_read_block() again and
> access mdev->mq while it rightfully belongs to the bus driver.  Bad
> things will happen.
> 
> So these two patches try to close the race windows.  Please note the
> FIXME in the write patch - I didn't do all the work.  Real life calls
> for attention, so these will be the last patches for a while.
> 
> Jörn
> 

The problem is that the hardware does not support another callback. In
any case while you are right that the function might be called as second
time, in the original code it will sleep while waiting for the lock,
which is allocated per device.

On the second patch - aiui completions do an uninterruptible wait, that
means they are surely not a good choice for this - especially as the
device could be unplugged at any time. (Admittedly my documentation
migght be of date here)

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