lkml.org 
[lkml]   [2002]   [Jan]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [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/
    \
     
     \ /
      Last update: 2005-03-22 13:15    [W:0.028 / U:2.396 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site