Ext2 xattr error handling

Ext2 xattr error handling

[Jan Kara]

  Hi!

  I think there's a subtle bug in handling of errors when
creating xattrs in ext2. In ext2_xattr_set2() if inode is marked for SYNC
ext2_sync_inode() is called. It can fail and then the state of xattrs is
inconsistent (actually it depends on the place at which the sync fails -
if inode itself cannot be written then it's probably an IO error and we
don't care to much but sync can fail also because of ENOSPC and in such
case we should proceed more gracefully). The attached patch should fix
that.
								Honza

-- 
Jan Kara <jack@suse.cz>
SuSE CR Labs


Fix a subtle bug in error handling. When ext2_sync_inode() fails because
of ENOSPC we want to keep xattrs in consistent state and release the old
block properly.

Signed-off-by: Jan Kara <jack@suse.cz>

--- linux-2.6-linus/fs/ext2/xattr.c	2005-01-24 12:52:18.000000000 +0100
+++ linux-2.6-linus/fs/ext2/xattr.c	2005-01-24 14:19:49.635103592 +0100
@@ -706,8 +706,11 @@
 	inode->i_ctime = CURRENT_TIME_SEC;
 	if (IS_SYNC(inode)) {
 		error = ext2_sync_inode (inode);
-		if (error)
+		if (error != ENOSPC)
 			goto cleanup;
+		/* In case sync failed due to ENOSPC the inode was actually
+		 * written (only some dirty data were not) so we just proceed
+		 * as if nothing happened and cleanup the unused block */
 	} else
 		mark_inode_dirty(inode);
 

Re: Ext2 xattr error handling

[Andreas Gruenbacher]

On Mon, 2005-01-24 at 12:22, Jan Kara wrote:
>   Hi!
> 
>   I think there's a subtle bug in handling of errors when
> creating xattrs in ext2. In ext2_xattr_set2() if inode is marked for SYNC
> ext2_sync_inode() is called. It can fail and then the state of xattrs is
> inconsistent (actually it depends on the place at which the sync fails -
> if inode itself cannot be written then it's probably an IO error and we
> don't care to much but sync can fail also because of ENOSPC and in such
> case we should proceed more gracefully). The attached patch should fix
> that.

It should probably just do this, no?


Index: linux-2.6.11-latest/fs/ext2/xattr.c
===================================================================
--- linux-2.6.11-latest.orig/fs/ext2/xattr.c
+++ linux-2.6.11-latest/fs/ext2/xattr.c
@@ -705,7 +705,7 @@ ext2_xattr_set2(struct inode *inode, str
 	EXT2_I(inode)->i_file_acl = new_bh ? new_bh->b_blocknr : 0;
 	inode->i_ctime = CURRENT_TIME_SEC;
 	if (IS_SYNC(inode)) {
-		error = ext2_sync_inode (inode);
+		error = ext2_write_inode(inode, 1);
 		if (error)
 			goto cleanup;
 	} else


Cheers,
-- 
Andreas Gruenbacher <agruen@suse.de>
SUSE Labs, SUSE LINUX GMBH