linux-kernel - [PATCH 00/30] Remove iget() and read

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 01 Oct 2007 14:09:21 +0100
From:	David Howells <dhowells@...hat.com>
To:	hch@...radead.org, viro@....linux.org.uk, torvalds@...l.org,
	akpm@...l.org
Cc:	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
	dhowells@...hat.com
Subject: [PATCH 00/30] Remove iget() and read_inode()

Hi Christoph, Al,

Here's a set of patches that remove all calls to iget() and all read_inode()
functions.  They should be removed for two reasons: firstly they don't lend
themselves to good error handling, and secondly their presence is a temptation
for code outside a filesystem to call iget() to access inodes within that
filesystem.

There are a few benefits to this:

 (1) Error handling gets simpler as you can return an error code rather than
     having to call is_bad_inode().

 (2) You can now tell the difference between ENOMEM and EIO occurring in the
     read_inode() path.

 (3) The code should get smaller.  iget() is an inline function that is
     typically called 2-3 times per filesystem that uses it.  By folding the
     iget code into the read_inode code for each filesystem, it eliminates
     some duplication.

A tarball of the patches can be retrieved from:

	http://people.redhat.com/~dhowells/iget-remove.tar.bz2

The first patch adds a function, iget_failed() that is a canned piece of code
for killing an inode when the inode construction path fails.

The second and third patches makes AFS and GFS2 use iget_failed() rather than
interpolating the sequence directly.

The fourth patch marks iget() and read_inode() as being deprecated.

The final patch removes iget() and read_inode() completely.

Each of the other patches modify a filesystem that used iget() and read_inode()
to use iget_locked() instead.  The standard procedure was to convert:

	void thingyfs_read_inode(struct inode *inode)
	{
		...
	}

into:

	struct inode *thingyfs_iget(struct super_block *sp, unsigned long ino)
	{
		struct inode *inode;
		int ret;

		inode = iget_locked(sb, ino);
		if (!inode)
			return ERR_PTR(-ENOMEM);
		if (!(inode->i_state & I_NEW))
			return inode;

		...
		unlock_new_inode(inode);
		return inode;
	error:
		iget_failed(inode);
		return ERR_PTR(ret);
	}

and then call thingyfs_iget() rather than iget():

	ret = -EINVAL;
	inode = iget(sb, ino);
	if (!inode || is_bad_inode(inode))
		goto error;

becomes:

	inode = thingyfs_iget(sb, ino);
	if (IS_ERR(inode)) {
		ret = PTR_ERR(inode);
		goto error;
	}

There were exceptions; most notably it appeared FAT should be calling ilookup()
not iget().

Additionally, HPPFS and HOSTFS (UM-specific filesystems) really need checking:

 hostfs_kern.c:

 (*) hostfs_iget() should perhaps subsume init_inode() and hostfs_read_inode().

 (*) It would appear that all hostfs inodes are the same inode because iget()
     was being called with inode number 0 - which forms the lookup key.

 hppfs_kern.c:

 (*) The HPPFS inode retains a pointer to the proc dentry it is shadowing, but
     whilst it does appear to retain a reference to it, it doesn't appear to
     destroy the reference if the inode goes away.

 (*) hppfs_iget() should perhaps subsume init_inode() and hppfs_read_inode().

 (*) It would appear that all hppfs inodes are the same inode because iget()
     was being called with inode number 0, which forms the lookup key.

David
-
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/