linux-kernel - Re: [PATCH] nfs: init req_lock in nfs_alloc

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:	Tue, 20 Feb 2007 19:37:18 -0500
From:	Trond Myklebust <trond.myklebust@....uio.no>
To:	Olof Johansson <olof@...om.net>
Cc:	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] nfs: init req_lock in nfs_alloc_inode

On Tue, 2007-02-20 at 11:23 -0600, Olof Johansson wrote:

> In my original reproduction, I had to boot with nfs root, and try to mount
> my sata drive (/dev/sda3). This is with a static /dev, no udev. Seems like it
> happens when trying to mount any block device that's located on NFS.
> 
> Since this is what nfs_sync_mapping_wait does:
> 
> long nfs_sync_mapping_wait(struct address_space *mapping, struct
> 		 	   writeback_control *wbc, int how) {
>         struct inode *inode = mapping->host;
>         struct nfs_inode *nfsi = NFS_I(inode);
> [...]
>         spin_lock(&nfsi->req_lock);
> [...]
> 
> I added this and it pops when mounting:
> 
> @@ -421,6 +421,10 @@ int nfs_getattr(struct vfsmount *mnt, st
>         int need_atime = NFS_I(inode)->cache_validity & NFS_INO_INVALID_ATIME;
>         int err;
>  
> +       if (inode->i_mapping->host != inode) {
> +               printk("inode %p host %p\n", inode, inode->i_mapping->host);
> +               printk("inode_nfs %p host_nfs %p\n", NFS_I(inode), NFS_I(inode->i_mapping->host));
> +       }
>         /* Flush out writes to the server in order to update c/mtime */
>         nfs_sync_mapping_range(inode->i_mapping, 0, 0, FLUSH_NOCOMMIT);
>  
> 
> I don't claim to know VFS internals, but doesn't it make sense that the
> device node is backed against the actual device, not an NFS inode? And
> if so, NFS can't expect to do nfs_sync_mapping_range() on it, or at
> least not dereference ->host and use it as an NFS inode, right?

NFS still has to manage the inode attributes and handle permissions. It
is only when you open the device that the VFS takes over (see the call
to init_special_inode() in nfs_fhget()).

> What I'm not sure I understand is why it disappears in the first place
> when I add the spin lock init -- I never even see the i_mapping->host
> pointer being allocated as an nfs inode. Maybe I just messed that one
> up somehow.

Looks like we need a check in nfs_getattr() for a regular file. It makes
no sense to call nfs_sync_mapping_range() on anything else. I think that
should fix your problem: it will stop the NFS client from interfering
with dirty pages on that inode's mapping.

Cheers
  Trond

Download attachment "linux-2.6.20-000-fix_block_device_getattr.dif" of type "message/rfc822" (968 bytes)