Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

jffs2: correct logic when creating a hole in jffs2_write_begin

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: Yifei Liu <yifeliu@cs.stonybrook.edu>
Signed-off-by: Erez Zadok <ezk@cs.stonybrook.edu>
Signed-off-by: Manish Adkar <madkar@cs.stonybrook.edu>
Signed-off-by: Richard Weinberger <richard@nod.at>

authored by

Yifei Liu and committed by
Richard Weinberger
23892d38 2241ab53

+7 -8
+7 -8
fs/jffs2/file.c
··· 137 137 struct jffs2_inode_info *f = JFFS2_INODE_INFO(inode); 138 138 struct jffs2_sb_info *c = JFFS2_SB_INFO(inode->i_sb); 139 139 pgoff_t index = pos >> PAGE_SHIFT; 140 - uint32_t pageofs = index << PAGE_SHIFT; 141 140 int ret = 0; 142 141 143 142 jffs2_dbg(1, "%s()\n", __func__); 144 143 145 - if (pageofs > inode->i_size) { 146 - /* Make new hole frag from old EOF to new page */ 144 + if (pos > inode->i_size) { 145 + /* Make new hole frag from old EOF to new position */ 147 146 struct jffs2_raw_inode ri; 148 147 struct jffs2_full_dnode *fn; 149 148 uint32_t alloc_len; 150 149 151 - jffs2_dbg(1, "Writing new hole frag 0x%x-0x%x between current EOF and new page\n", 152 - (unsigned int)inode->i_size, pageofs); 150 + jffs2_dbg(1, "Writing new hole frag 0x%x-0x%x between current EOF and new position\n", 151 + (unsigned int)inode->i_size, (uint32_t)pos); 153 152 154 153 ret = jffs2_reserve_space(c, sizeof(ri), &alloc_len, 155 154 ALLOC_NORMAL, JFFS2_SUMMARY_INODE_SIZE); ··· 168 169 ri.mode = cpu_to_jemode(inode->i_mode); 169 170 ri.uid = cpu_to_je16(i_uid_read(inode)); 170 171 ri.gid = cpu_to_je16(i_gid_read(inode)); 171 - ri.isize = cpu_to_je32(max((uint32_t)inode->i_size, pageofs)); 172 + ri.isize = cpu_to_je32((uint32_t)pos); 172 173 ri.atime = ri.ctime = ri.mtime = cpu_to_je32(JFFS2_NOW()); 173 174 ri.offset = cpu_to_je32(inode->i_size); 174 - ri.dsize = cpu_to_je32(pageofs - inode->i_size); 175 + ri.dsize = cpu_to_je32((uint32_t)pos - inode->i_size); 175 176 ri.csize = cpu_to_je32(0); 176 177 ri.compr = JFFS2_COMPR_ZERO; 177 178 ri.node_crc = cpu_to_je32(crc32(0, &ri, sizeof(ri)-8)); ··· 201 202 goto out_err; 202 203 } 203 204 jffs2_complete_reservation(c); 204 - inode->i_size = pageofs; 205 + inode->i_size = pos; 205 206 mutex_unlock(&f->sem); 206 207 } 207 208