• Yifei Liu's avatar
    jffs2: correct logic when creating a hole in jffs2_write_begin · 23892d38
    Yifei Liu authored
    Bug description and fix:
    
    1. Write data to a file, say all 1s from offset 0 to 16.
    
    2. Truncate the file to a smaller size, say 8 bytes.
    
    3. Write new bytes (say 2s) from an offset past the original size of the
    file, say at offset 20, for 4 bytes.  This is supposed to create a "hole"
    in the file, meaning that the bytes from offset 8 (where it was truncated
    above) up to the new write at offset 20, should all be 0s (zeros).
    
    4. Flush all caches using "echo 3 > /proc/sys/vm/drop_caches" (or unmount
    and remount) the f/s.
    
    5. Check the content of the file.  It is wrong.  The 1s that used to be
    between bytes 9 and 16, before the truncation, have REAPPEARED (they should
    be 0s).
    
    We wrote a script and helper C program to reproduce the bug
    (reproduce_jffs2_write_begin_issue.sh, write_file.c, and Makefile).  We can
    make them available to anyone.
    
    The above example is shown when writing a small file within the same first
    page.  But the bug happens for larger files, as long as steps 1, 2, and 3
    above all happen within the same page.
    
    The problem was traced to the jffs2_write_begin code, where it goes into an
    'if' statement intended to handle writes past the current EOF (i.e., writes
    that may create a hole).  The code computes a 'pageofs' that is the floor
    of the write position (pos), aligned to the page size boundary.  In other
    words, 'pageofs' will never be larger than 'pos'.  The code then sets the
    internal jffs2_raw_inode->isize to the size of max(current inode size,
    pageofs) but that is wrong: the new file size should be the 'pos', which is
    larger than both the current inode size and pageofs.
    
    Similarly, the code incorrectly sets the internal jffs2_raw_inode->dsize to
    the difference between the pageofs minus current inode size; instead it
    should be the current pos minus the current inode size.  Finally,
    inode->i_size was also set incorrectly.
    
    The patch below fixes this bug.  The bug was discovered using a new tool
    for finding f/s bugs using model checking, called MCFS (Model Checking File
    Systems).
    Signed-off-by: default avatarYifei Liu <yifeliu@cs.stonybrook.edu>
    Signed-off-by: default avatarErez Zadok <ezk@cs.stonybrook.edu>
    Signed-off-by: default avatarManish Adkar <madkar@cs.stonybrook.edu>
    Signed-off-by: default avatarRichard Weinberger <richard@nod.at>
    23892d38
file.c 9.37 KB