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

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

[Stephen C. Tweedie]

[-- 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;

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

[Andreas Gruenbacher]

Hello,

I agree with Stephen that the revised symlink checking code should be
added to the 2.4 series. Marcelo, could you please apply?

(The extended attribute patches contain an equivalent check, but these
patches will not get integrated into 2.4, so people switching between
vanilla 2.4 and 2.6 without this patch would run into the problems
mentioned. Because I wasn't using EA-unaware kernels much since ages, I
didn't experience this problem.)


On Mon, 2003-12-22 at 15:43, Stephen C. Tweedie wrote:
> 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?

I don't think we should. EA file systems work "well enough" on
EA-unaware 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.

The only problem that deleting EA inodes with a non-EA-aware kernel
causes are refcounts that are too high, and a file system check will fix
this. When no FS check is done, a few file system blocks that appear to
be in use actually won't be. I think this event will be rare, so I would
not introduce a read-only flag.


Thanks,
-- 
Andreas Gruenbacher <agruen@suse.de>
SUSE Labs, SUSE LINUX AG