On Wed, 2007-05-23 at 21:16 -0400, Erez Zadok wrote:
> I've hit a NULL ptr deref on desc->pg_error below, triggered when mounting a
> stackable file system on top of nfsv3:
>
> // from file: nfs/pagelist.c
> int nfs_pageio_add_request(struct nfs_pageio_descriptor *desc,
> struct nfs_page *req)
> {
> while (!nfs_pageio_do_add_request(desc, req)) {
> nfs_pageio_doio(desc);
> if (desc->pg_error < 0)
>
> Scenario:
>
> 2.6.22-rc2 with Unionfs 2.0 (release u2 for 2.6.22-rc2, which includes mmap
> support).
>
> I mount unionfs on top of nfs (v3). I have one file in the nfs branch. I
> run a simple program through the union which mmap's the file, changes the
> first byte of the file, calls msync(), and then closes. This causes
> unionfs_writepage to be invoked, which in turn calls the lower file system's
> ->writepage, here nfs_writepage.
>
> The 'wbc' that's passed to unionfs_writepage from the VFS has this:
>
> wbc->for_writepages = 1
> wbc->fs_private = NULL
>
> If you follow the logic, then nfs_writepage calls nfs_writepage_locked,
> passing the same wbc. nfs_writepage_locked does this:
>
> if (wbc->for_writepages)
> pgio = wbc->fs_private;
> else {
> nfs_pageio_init_write(&mypgio, inode, wb_priority(wbc));
> pgio = &mypgio;
> }
>
> which means that pgio is set to NULL from the caller's wbc. Then
> nfs_writepage_locked calls nfs_page_async_flush, passing it this pgio
> (NULL). nfs_page_async_flush invokes nfs_pageio_add_request, passing it
> this NULL pgio. Inside nfs_pageio_add_request the NULL is being
> dereferenced as desc->pg_error and we get an oops.
>
> As a workaround, in unionfs_writepage I tried this before calling the lower
> file system's ->writepage (which was nfs_writepage):
>
> struct writeback_control lower_wbc;
> memcpy(&lower_wbc, wbc, sizeof(struct writeback_control));
> if (lower_wbc.for_writepages && !lower_wbc.fs_private) {
> printk("unionfs: setting wbc.for_writepages to 0\n");
> lower_wbc.for_writepages = 0;
> }
>
> Then I passed &lower_wbc to the lower file system's writepage method
> (nfs_writepage). It works; no oops, and the file in question was sync'ed to
> the backing f/s too. But I'm not sure if it's the correct workaround and
> whether it'd break things for other non-NFS file systems.
>
> It's possible that I'm doing something wrong in unionfs's mmap code, which
> indirectly results in a malformed wbc structure being passed to unionfs (by
> malformed I mean that wbc->fs_private is NULL and wbc->for_writepages is set
> to 1). If such a wbc can be created by any other means and passed to NFS,
> then nfs probably will continue to oops even w/o unionfs.
>
> FWIW, I tried a similar scenario with eCryptfs (another stackable f/s in
> 2.6.22-rc2) on top of NFSv3, and got the same oops (sorry, Mike :-)
>
> Any pointers would be appreciated.
If this is truly a call to ->writepages() by the VFS (as opposed to a
call to ->writepage()) then why is unionfs' writepages() failing to call
the underlying writepages method of the host filesystem: in this case
nfs_writepages()?
Trond
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
