[PATCH] 2.2: Consistency in ext2 error setting

From: Chip Salzenberg (chip@valinux.com)
Date: Tue Aug 29 2000 - 20:16:55 EST


This is a patch from Andreas Dilger <adilger@turbolabs.com>.
It claims to fix some corner cases where an error code may not be
returned properly. Here's the description:

--------------------------------------------------------------------------
ext2_bread always sets err to some sane value. I traced this through:

ext2_bread() calls ext2_getblk to set *err or sets *err = -EIO;
ext2_getblk() calls ext2_get_block to set *err;
ext2_get_block() returns 0, -EIO, or {inode,block}_getblk _may_ set err;

The inode_getblk and block_getblk functions themselves do not always set *err,
although err = 0 before they are called from ext2_get_block(), so there is
_always_ a return value set from ext2_get_block().

The ext2_get_block() function may call block_getblk() several times without
checking the return value, so it _appears_ it would overwrite the last error
value. However, one (non-obvious) thing to note is that bh = NULL on error
return* from {inode,block}_getblk(), and calls to block_getblk() are a no-op
if bh == NULL.

(*) There was one (probably very rare) error case where block_getblk()
might return non-NULL: after bforget(result) jumps back to repeat: and
then we get an error with ext2_alloc_block() - we return a bad result.
I also fixed this in the attached patch.

I also found that ext2_alloc_block() did not set a return code in the
(#undef'd) PREALLOC path, and inode_getblk() set a return value instead
of using the error from ext2_alloc_block(). I also verified that
ext2_alloc_block() itself always sets an error code.
--------------------------------------------------------------------------

Index: fs/ext2/balloc.c
--- fs/ext2/balloc.c.prev
+++ fs/ext2/balloc.c Sat Jun 17 17:48:19 2000
@@ -479,9 +479,6 @@ repeat:
                         i = 0;
                 gdp = ext2_get_group_desc (sb, i, &bh2);
- if (!gdp) {
- *err = -EIO;
- unlock_super (sb);
- return 0;
- }
+ if (!gdp)
+ goto io_error;
                 if (le16_to_cpu(gdp->bg_free_blocks_count) > 0)
                         break;

Index: fs/ext2/inode.c
--- fs/ext2/inode.c.prev
+++ fs/ext2/inode.c Sat Jun 17 17:48:19 2000
@@ -132,4 +132,5 @@ static int ext2_alloc_block (struct inod
                 mark_buffer_dirty(bh, 1);
                 brelse (bh);
+ *err = 0;
         } else {
                 ext2_discard_prealloc (inode);
@@ -207,4 +208,5 @@ int ext2_bmap (struct inode * inode, int
 }
 
+/* must return NULL on error */
 static struct buffer_head * inode_getblk (struct inode * inode, int nr,
                                           int create, int new_block, int * err)
@@ -219,5 +221,5 @@ repeat:
         tmp = *p;
         if (tmp) {
- struct buffer_head * result = getblk (inode->i_dev, tmp, inode->i_sb->s_blocksize);
+ result = getblk (inode->i_dev, tmp, inode->i_sb->s_blocksize);
                 if (tmp == *p)
                         return result;

-- 
Chip Salzenberg              - a.k.a. -              <chip@valinux.com>
"I wanted to play hopscotch with the impenetrable mystery of existence,
    but he stepped in a wormhole and had to go in early."  // MST3K
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Thu Aug 31 2000 - 21:00:24 EST