On Wed, 2010-12-01 at 17:03 -0800, Frank Filz wrote:
>
>
> I discovered this problem by accident while doing some testing of the
> Ganesha user space server. It was producing garbage fileids that
> happened to have bit 31 set (0x80000000).
>
> The telldir special test from cthon04 would fail. Investigating, I found
> that it appeared that the getdents() was returning EOVERFLOW. It wasn't
> too hard to track that down to the following code:
>
> static int fillonedir(void * __buf, const char * name, int namlen, loff_t offset,
> u64 ino, unsigned int d_type)
> {
> struct readdir_callback * buf = (struct readdir_callback *) __buf;
> struct old_linux_dirent __user * dirent;
> unsigned long d_ino;
>
> if (buf->result)
> return -EINVAL;
> d_ino = ino;
> if (sizeof(d_ino) < sizeof(ino) && d_ino != ino)
> return -EOVERFLOW;
>
> It took adding some debug code to track the problem down to this
> function:
>
> u64 nfs_compat_user_ino64(u64 fileid)
> {
> int ino;
>
> if (enable_ino64)
> return fileid;
> ino = fileid;
> if (sizeof(ino) < sizeof(fileid))
> ino ^= fileid >> (sizeof(fileid)-sizeof(ino)) * 8;
> return ino;
> }
>
> In trying to reduce a 64 bit fileid to 32 bits, it produces a SIGNED 32
> bit int! When this is passed to fillonedir as a uint64, a negative
> number is sign extended. bit 31 of ino will be set if bit 31 OR bit 63
> (but not both) is set in the fileid.
>
> Turns out the fix is simple! Change ino to an unsigned int.
>
> In order to test my fix in an orderly fashion, I used a simple process
> to modify the fileids produced by the kernel server:
>
> u64 warp_fileid(u64 fileid)
> {
> return (fileid & 0xffffffff7fffffefLL) | ((fileid & 0x10LL) << 27) | ((fileid &0x80000000LL) >> 27);
> }
>
> This means that every 16 inode numbers, bit 31 will be flipped,
> producing plenty of problem fileids. The telldir test case fails with
> this hacked kernel server.
>
> Of course if anyone has a real file system with > 2G inodes, they could
> see the problem for real, but I don't have a big enough file system...
>
>
>
>
>
> Signed-off-by: Frank Filz <ffilzlnx@xxxxxxxxxx>
> ---
> diff -X ignore.patcher -ruNp linux-2.6.18-194.el5/fs/nfs/inode.c linux-2.6.18-194.ff/fs/nfs/inode.c
> --- linux-2.6.18-194.el5/fs/nfs/inode.c 2010-12-01 15:52:11.000000000 -0800
> +++ linux-2.6.18-194.ff/fs/nfs/inode.c 2010-12-01 16:53:28.000000000 -0800
> @@ -71,7 +71,7 @@ static kmem_cache_t * nfs_inode_cachep;
> */
> u64 nfs_compat_user_ino64(u64 fileid)
> {
> - int ino;
> + unsigned int ino;
Shouldn't this just be of type 'compat_ulong_t' if CONFIG_COMPAT is
defined, and of type 'unsigned long' if not?
Cheers
Trond
--
Trond Myklebust
Linux NFS client maintainer
NetApp
Trond.Myklebust@xxxxxxxxxx
www.netapp.com
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
