[PATCH] SMP race in ext2 - metadata corruption.

Alexander Viro (viro@math.psu.edu)
Thu, 26 Apr 2001 11:45:47 -0400 (EDT)


Ext2 does getblk+wait_on_buffer for new metadata blocks before
filling them with zeroes. While that is enough for single-processor,
on SMP we have the following race:

getblk gives us unlocked, non-uptodate bh
wait_on_buffer() does nothing
read from device locks it and starts IO
we zero it out.
on-disk data overwrites our zeroes.
we mark it dirty
bdflush writes the old data (_not_ zeroes) back to disk.

Result: crap in metadata block. Proposed fix: lock_buffer()/unlock_buffer()
around memset()/mark_buffer_uptodate() instead of wait_on_buffer() before
them.

Patch against 2.4.4-pre7 follows. Please, apply.
Al

--- S4-pre7/fs/ext2/inode.c Wed Apr 25 20:43:08 2001
+++ S4-pre7-ext2/fs/ext2/inode.c Thu Apr 26 11:36:11 2001
@@ -397,13 +397,13 @@
* the pointer to new one, then send parent to disk.
*/
bh = getblk(inode->i_dev, parent, blocksize);
- if (!buffer_uptodate(bh))
- wait_on_buffer(bh);
+ lock_buffer(bh);
memset(bh->b_data, 0, blocksize);
branch[n].bh = bh;
branch[n].p = (u32*) bh->b_data + offsets[n];
*branch[n].p = branch[n].key;
mark_buffer_uptodate(bh, 1);
+ unlock_buffer(bh);
mark_buffer_dirty_inode(bh, inode);
if (IS_SYNC(inode) || inode->u.ext2_i.i_osync) {
ll_rw_block (WRITE, 1, &bh);
@@ -587,10 +587,10 @@
struct buffer_head *bh;
bh = getblk(dummy.b_dev, dummy.b_blocknr, inode->i_sb->s_blocksize);
if (buffer_new(&dummy)) {
- if (!buffer_uptodate(bh))
- wait_on_buffer(bh);
+ lock_buffer(bh);
memset(bh->b_data, 0, inode->i_sb->s_blocksize);
mark_buffer_uptodate(bh, 1);
+ unlock_buffer(bh);
mark_buffer_dirty_inode(bh, inode);
}
return bh;

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/