2.2.x BUG? ext2/file.c ext2_file_write - unnecessary copy_from_user retry

From: Roger Larsson (roger.larsson@norran.net)
Date: Thu Mar 30 2000 - 16:15:00 EST


Hi,

I got no reaction last time, I try to resend it with a better heading.

from ext2/file.c(280..307) linux-2.2.13 (2.2.14 looks the same)
interesting lines marked (a)..(d)

                /* Tricky: what happens if we are writing the complete
                 * contents of a block which is not currently
                 * initialized? We have to obey the same
                 * synchronisation rules as the IO code, to prevent some
                 * other process from stomping on the buffer contents by
                 * refreshing them from disk while we are setting up the
                 * buffer. The copy_from_user() can page fault, after
                 * all. We also have to throw away partially successful
                 * copy_from_users to such buffers, since we can't trust
                 * the rest of the buffer_head in that case. --sct */

(a) new_buffer = (!buffer_uptodate(bh) && !buffer_locked(bh)
&&
                              c == sb->s_blocksize);

(b) if (new_buffer) {
                        set_bit(BH_Lock, &bh->b_state);
(c) c -= copy_from_user (bh->b_data + offset, buf,
c);
(d) if (c != sb->s_blocksize) {
                                c = 0;
                                unlock_buffer(bh);
                                brelse(bh);
                                if (!written)
                                        written = -EFAULT;
                                break;
                        }
                        mark_buffer_uptodate(bh, 1);
                        unlock_buffer(bh);
                } else {
                  

(a) Only buffers with "c == sb->s_blocksize" can be new_buffers
(b) Test for new buffers
(c.1) copy_from_user(..., c) will return c in most occasions, rarely 0.
(c.2) c -= c gives c = 0, c -= 0 gives c = sb->s_b
(d) if (c != sb->s_blocksize)

results in:
* new block results in retry of copy_from_user even if it succeed the
  first time (waste!)
* new block results in retry of copy_from_user with partial success (ok)
* new block assumes success when nothing was copied (bug, but can it
happen?)

should not the test in (d) be
 
        if (c > 0)

to catch partial copies?

/RogerL

--
Home page:
  http://www.norran.net/nra02596/

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



This archive was generated by hypermail 2b29 : Fri Mar 31 2000 - 21:00:27 EST