[patch] linux-2.4 Ext2/3 compatibility problem with EAs on symlinks

["Stephen C. Tweedie" <sct@redhat.com>]

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

Hi all, 

I found why people running SELinux on 2.6 were having trouble booting
again on 2.4 kernels, and it's nothing to do with SELinux per-se.  It's
a compatibility problem with extended attributes.  

The trouble is that setting an EA on a fast symlink upsets the kernel's
symlink detection code.  Older kernels will see a symlink with non-zero
i_blocks, and will assume that the symlink is a slow one --- ie. that
the first direct block of the inode points to the symlink contents ---
and will end up doing a block lookup from what is in fact the first four
ascii chars from the symlink path.

2.6 kernels deal with this by detecting a non-zero i_file_acl field and
subtracting the EA's blocks from i_blocks before checking if the block
count is zero.  The patch below adds the same compatibility code to
2.4.  Booting a SELinux partition results in immediate death via access
beyond end-of-device as soon as the kernel tries to dereference /bin/sh
(a symlink to /bin/bash on this box); with the fix, it's fine.

Any kernel built with EA patches will be immune to this, even if the EA
config is not set.  So, 2.6 is simply not vulnerable.

There are a couple of questions left over.  First, should we be adding a
compatibility flag so that mounting filesystems with EAs on symlinks is
rejected on older kernels?  Secondly, there's the problem of getting EA
refcounts out-of-sync if you delete an inode with EAs on an older,
non-EA-aware kernel.  Even with the fix, the old kernel will still fail
to update the refcount on the EA block so we'd need a fsck to fix things
up; setting a readonly compatibility bit when EAs are present would
avoid that, but it's not clear whether the underlying problem is bad
enough to justify the hit in usability.

Cheers,
 Stephen


[-- Attachment #2: 4208-ext23-symlink-ea-compat.patch --]
[-- Type: text/plain, Size: 2577 bytes --]

===== fs/ext3/inode.c 1.16 vs edited =====
--- linux-2.4-tmp/fs/ext2/inode.c.=K0000=.orig
+++ linux-2.4-tmp/fs/ext2/inode.c
@@ -35,6 +35,17 @@ MODULE_AUTHOR("Remy Card and others");
 MODULE_DESCRIPTION("Second Extended Filesystem");
 MODULE_LICENSE("GPL");
 
+/*
+ * Test whether an inode is a fast symlink.
+ */
+static inline int ext2_inode_is_fast_symlink(struct inode *inode)
+{
+	int ea_blocks = inode->u.ext2_i.i_file_acl ?
+		(inode->i_sb->s_blocksize >> 9) : 0;
+
+	return (S_ISLNK(inode->i_mode) &&
+		inode->i_blocks - ea_blocks == 0);
+}
 
 static int ext2_update_inode(struct inode * inode, int do_sync);
 
@@ -801,6 +812,8 @@ void ext2_truncate (struct inode * inode
 	if (!(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) ||
 	    S_ISLNK(inode->i_mode)))
 		return;
+	if (ext2_inode_is_fast_symlink(inode))
+		return;
 	if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
 		return;
 
@@ -1001,7 +1014,7 @@ void ext2_read_inode (struct inode * ino
 		inode->i_fop = &ext2_dir_operations;
 		inode->i_mapping->a_ops = &ext2_aops;
 	} else if (S_ISLNK(inode->i_mode)) {
-		if (!inode->i_blocks)
+		if (ext2_inode_is_fast_symlink(inode))
 			inode->i_op = &ext2_fast_symlink_inode_operations;
 		else {
 			inode->i_op = &page_symlink_inode_operations;
--- linux-2.4-tmp/fs/ext3/inode.c.=K0000=.orig
+++ linux-2.4-tmp/fs/ext3/inode.c
@@ -39,6 +39,18 @@
  */
 #undef SEARCH_FROM_ZERO
 
+/*
+ * Test whether an inode is a fast symlink.
+ */
+static inline int ext3_inode_is_fast_symlink(struct inode *inode)
+{
+	int ea_blocks = EXT3_I(inode)->i_file_acl ?
+		(inode->i_sb->s_blocksize >> 9) : 0;
+
+	return (S_ISLNK(inode->i_mode) &&
+		inode->i_blocks - ea_blocks == 0);
+}
+
 /* The ext3 forget function must perform a revoke if we are freeing data
  * which has been journaled.  Metadata (eg. indirect blocks) must be
  * revoked in all cases. 
@@ -1870,6 +1882,8 @@ void ext3_truncate(struct inode * inode)
 	if (!(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) ||
 	    S_ISLNK(inode->i_mode)))
 		return;
+	if (ext3_inode_is_fast_symlink(inode))
+		return;
 	if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
 		return;
 
@@ -2170,7 +2184,7 @@ void ext3_read_inode(struct inode * inod
 		inode->i_op = &ext3_dir_inode_operations;
 		inode->i_fop = &ext3_dir_operations;
 	} else if (S_ISLNK(inode->i_mode)) {
-		if (!inode->i_blocks)
+		if (ext3_inode_is_fast_symlink(inode))
 			inode->i_op = &ext3_fast_symlink_inode_operations;
 		else {
 			inode->i_op = &page_symlink_inode_operations;