Messages in this thread Patch in this message |  | | | Date | Sun, 06 Jan 2002 20:28:37 -0800 | | From | Andrew Morton <> | | Subject | Re: [patch] truncate fixes |
| |
Andrea Arcangeli wrote: > > > (I think I'll add a buffer_mapped() test to this code as well. It's > > a bit redundant because the fs shouldn't go setting BH_New and not > > BH_Mapped, but this code is _very_ rarely executed, and I haven't > > tested all filesystems...) > > correct, it shouldn't be necessary. I wouldn't add it. if a fs breaks the > buffer_new semantics it's the one that should be fixed methinks.
You mean "don't be lazy. Audit all the filesystems"? Sigh. OK.
> > > > @@ -1633,12 +1660,22 @@ static int __block_prepare_write(struct > > */ > > while(wait_bh > wait) { > > wait_on_buffer(*--wait_bh); > > - err = -EIO; > > if (!buffer_uptodate(*wait_bh)) > > - goto out; > > + return -EIO; > > } > > return 0; > > out: > > + bh = head; > > + block_start = 0; > > + do { > > + if (buffer_new(bh) && buffer_mapped(bh) && !buffer_uptodate(bh)) { > > + memset(kaddr+block_start, 0, bh->b_size); > > + set_bit(BH_Uptodate, &bh->b_state); > > + mark_buffer_dirty(bh); > > + } > > + block_start += bh->b_size; > > + bh = bh->b_this_page; > > + } while (bh != head); > > I found another problem, we really need to keep track of which bh are > been created by us during the failing prepare_write (buffer_new right > now, not a long time ago), or we risk to corrupt data with a write > passing over many bh, where the first bh of the page contained vaild > data since a long time ago. To do this: 1) we either keep track of it > on the kernel stack with some local variable or 2) we change > the buffer_new semantics so that they indicate an "instant buffer_new" > to clear just after checking it
Fair enough. How does this (untested) approach look?
@@ -1600,6 +1627,7 @@ static int __block_prepare_write(struct if (block_start >= to) break; if (!buffer_mapped(bh)) { + clear_bit(BH_New, &bh->b_state); err = get_block(inode, block, bh, 1); if (err) goto out; @@ -1633,12 +1661,30 @@ static int __block_prepare_write(struct */ while(wait_bh > wait) { wait_on_buffer(*--wait_bh); - err = -EIO; if (!buffer_uptodate(*wait_bh)) - goto out; + return -EIO; } return 0; out: + /* + * Zero out any newly allocated blocks to avoid exposing stale + * data. If BH_New is set, we know that the block was newly + * allocated in the above loop. + */ + bh = head; + block_start = 0; + do { + if (buffer_new(bh)) { + if (buffer_uptodate(bh)) + printk(KERN_ERR __FUNCTION__ + ": zeroing uptodate buffer!\n"); + memset(kaddr+block_start, 0, bh->b_size); + set_bit(BH_Uptodate, &bh->b_state); + mark_buffer_dirty(bh); + } + block_start += bh->b_size; + bh = bh->b_this_page; + } while (bh != head); return err; } - 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/
|  |