linux-kernel - Re: [PATCH 2/2] fat (exportfs): fix dentry reconnection

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] [day] [month] [year] [list]

Message-ID: <87vchtp9u6.fsf@devron.myhome.or.jp>
Date:	Thu, 12 Jul 2012 19:12:01 +0900
From:	OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>
To:	"Steven J. Magnani" <steve@...idescorp.com>
Cc:	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] fat (exportfs): fix dentry reconnection

"Steven J. Magnani" <steve@...idescorp.com> writes:

> Maintain an index of directory inodes by starting cluster,
> so that fat_get_parent() can return the proper cached inode rather than
> inventing one that cannot be traced back to the filesystem root.
>
> Add a new msdos/vfat binary mount option "nfs" so that FAT filesystems
> that are _not_ exported via NFS are not saddled with maintenance of an index
> they will never use.
>
> Finally, simplify NFS file handle generation and lookups. 
> An ext2-congruent implementation is adequate for FAT needs.

Looks good to me. Let's test with this.

Acked-by: OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>

> +static struct inode *fat_dget(struct super_block *sb, int i_logstart)
>  {

[...]

If options.nfs == 0, it may be better to warn once.

printk_once()

> +	head = sbi->dir_hashtable + fat_dir_hash(i_logstart);
> +	spin_lock(&sbi->dir_hash_lock);
> +	hlist_for_each_entry(i, _p, head, i_dir_hash) {
> +		BUG_ON(i->vfs_inode.i_sb != sb);
> +		if (i->i_logstart != i_logstart)
> +			continue;
> +		inode = igrab(&i->vfs_inode);
> +		if (inode)
> +			break;
> +	}
> +	spin_unlock(&sbi->dir_hash_lock);
> +	return inode;
>  }

[...]

> @@ -128,24 +89,13 @@ struct dentry *fat_get_parent(struct den
>  	struct super_block *sb = child_dir->d_sb;
>  	struct buffer_head *bh = NULL;
>  	struct msdos_dir_entry *de;
> -	loff_t i_pos;
> -	struct dentry *parent;
> -	struct inode *inode;
> -	int err;
> -
> -	lock_super(sb);
> +	struct inode *parent_inode = NULL;
>  
> -	err = fat_get_dotdot_entry(child_dir->d_inode, &bh, &de, &i_pos);
> -	if (err) {
> -		parent = ERR_PTR(err);
> -		goto out;
> +	if (!fat_get_dotdot_entry(child_dir->d_inode, &bh, &de)) {
> +		int parent_logstart = fat_get_start(MSDOS_SB(sb), de);
> +		parent_inode = fat_dget(sb, parent_logstart);
>  	}

BTW, I guess, in future, if it is improve, we might be able to create
that "private" inode here. And register it to i_dir_hash without i_pos,
and detach while using by ->get_parent(). And fat_attach() side can find
and grab it, and update i_pos by real one.

Thanks.
-- 
OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>
--
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/