[patch] Fix block device inode list corruptions

[patch] Fix block device inode list corruptions

[Stephen C. Tweedie]

[-- Attachment #1: Type: text/plain, Size: 1261 bytes --]

Hi,

I've been chasing a weird SELinux bug which shows up mostly when doing
installs of a dev-* rpm (ie. creating and overwriting lots of block
device inodes), but which I've also seen when doing mkinitrd.

It turned out not to be an SELinux problem at all, but a core VFS
S_ISBLK bug.  It seems that SELinux simply widens the race window.

The code at fault is fs/fs-writeback.c:__mark_inode_dirty():

		/*
		 * Only add valid (hashed) inodes to the superblock's
		 * dirty list.  Add blockdev inodes as well.
		 */
		if (!S_ISBLK(inode->i_mode)) {
			if (hlist_unhashed(&inode->i_hash))
				goto out;
			if (inode->i_state & (I_FREEING|I_CLEAR))
				goto out;
		}

The "I_FREEING|I_CLEAR" condition was added after the ISBLK/unhashed
tests were already in the source, but I can't see any reason why we'd
want the I_FREEING test not to apply to block devices.  And indeed, this
results in all sorts of inode list corruptions.  Simply moving the
I_FREEING|I_CLEAR test out of the protection of the S_ISBLK() condition
fixes things entirely.  

The exiting 2.6 kernel will reliably fail on me in about 2 seconds once
"rpm -Uvh --force dev*.rpm" starts its actual installation of the new
inodes.  With the patch below I can't reproduce it at all.

--Stephen






[-- Attachment #2: vfs-blkdev-list.patch --]
[-- Type: text/plain, Size: 583 bytes --]

--- linux-2.6.1/fs/fs-writeback.c.=K0001=.orig
+++ linux-2.6.1/fs/fs-writeback.c
@@ -87,12 +87,11 @@ void __mark_inode_dirty(struct inode *in
 		 * Only add valid (hashed) inodes to the superblock's
 		 * dirty list.  Add blockdev inodes as well.
 		 */
-		if (!S_ISBLK(inode->i_mode)) {
+		if (!S_ISBLK(inode->i_mode))
 			if (hlist_unhashed(&inode->i_hash))
 				goto out;
-			if (inode->i_state & (I_FREEING|I_CLEAR))
-				goto out;
-		}
+		if (inode->i_state & (I_FREEING|I_CLEAR))
+			goto out;
 
 		/*
 		 * If the inode was already on s_dirty or s_io, don't

Re: [patch] Fix block device inode list corruptions

[viro]

On Wed, Feb 04, 2004 at 03:27:44PM +0000, Stephen C. Tweedie wrote:
> Hi,
> 
> I've been chasing a weird SELinux bug which shows up mostly when doing
> installs of a dev-* rpm (ie. creating and overwriting lots of block
> device inodes), but which I've also seen when doing mkinitrd.
> 
> It turned out not to be an SELinux problem at all, but a core VFS
> S_ISBLK bug.  It seems that SELinux simply widens the race window.
> 
> The code at fault is fs/fs-writeback.c:__mark_inode_dirty():
> 
> 		/*
> 		 * Only add valid (hashed) inodes to the superblock's
> 		 * dirty list.  Add blockdev inodes as well.
> 		 */
> 		if (!S_ISBLK(inode->i_mode)) {
> 			if (hlist_unhashed(&inode->i_hash))
> 				goto out;
> 			if (inode->i_state & (I_FREEING|I_CLEAR))
> 				goto out;
> 		}
> 
> The "I_FREEING|I_CLEAR" condition was added after the ISBLK/unhashed
> tests were already in the source, but I can't see any reason why we'd
> want the I_FREEING test not to apply to block devices.  And indeed, this
> results in all sorts of inode list corruptions.  Simply moving the
> I_FREEING|I_CLEAR test out of the protection of the S_ISBLK() condition
> fixes things entirely.  

ACK.