Re: CONFIRMED bug in do_generic_file_read

[Al Viro]

On Thu, May 12, 2005 at 05:57:57PM -0700, Jim Washer wrote:
> Well, my original post got ZERO responses, but I've continued to look 
> at this, and with the help of a colleague I think we've bottomed out.

> do_generic_file_read does indeed have a bug, as mentioned below. However, 
> this bug is generally not hit as the call to a_ops->readpage (normally 
> pointing at blkdev_readpage) NEVER returns an error. However, the Veritas 
> code's readpage does return errors, and this triggers this KERNEL BUG.

Hardly.  What it does, apparently, is extra page_cache_release().
 
> I don't know enough about the page cache to know if simply NULLing the 
> page->mapping pointer is safe.

Analysis above is BS.
	a) page with non-NULL ->mapping == page in cache.  Code that adds
page to page cache grabs a reference before putting it on the lists.
	b) code that evicts page from cache drops a reference after getting
the sucker off lists.
	c) final page_cache_release() while page is still in cache is,
indeed, a bug.  Of memory-corrupting variety.
	d) ->readpage() has no fscking business playing with any of the above.
	e) while we are at it, -EFAULT is a hell of a strange error for
->readpage(), but that's a separate story.

Check for unbalanced page_cache_release() in their code.

> Anyone care to address this?

See above.  Anything beyond that requires
	1) access to their code
	2) good supply of industrial-strength barf-bags to deal with
inevitable consequences of (1).
-
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/